This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] warning about const multidimensional array as function parameter
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Martin Uecker <uecker at eecs dot berkeley dot edu>
- Cc: gcc Mailing List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 29 Oct 2014 16:43:16 +0000
- Subject: Re: [PATCH] warning about const multidimensional array as function parameter
- Authentication-results: sourceware.org; auth=none
- References: <20141025123245 dot 4ed09551 at lemur> <Pine dot LNX dot 4 dot 64 dot 1410271254470 dot 13536 at digraph dot polyomino dot org dot uk> <20141028231040 dot 38b1cbc0 at lemur>
On Tue, 28 Oct 2014, Martin Uecker wrote:
> attached is a revised and extended patch. Changes with respect
> to the previous patch are:
Thanks for the revised patch. I've moved this to gcc-patches as the more
appropriate mailing list for discussion of specific patches as opposed to
more general questions. It would also be a good idea to get started on
the paperwork
http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
if you haven't already.
> Note that there is now a semantic (and not only diagnostic) change.
> Without this patch
>
> const int a[1];
> int b[1];
> (x ? &a : &b)
>
> would return a 'void*' and a warning about pointer type mismatch.
> With this patch the conditional has type 'const int (*)[1]'.
I believe that is safe (in that that conditional expression isn't valid in
ISO C). What wouldn't be safe is making a conditional expression between
void * and const int (*)[] have type const void * instead of void *.
> * c-typeck.c: New behavior for pointers to arrays with qualifiers
Note that the ChangeLog entry should name the functions being changed and
what changed in each function (it's also helpful to diff with "svn diff -x
-up" so that the function names are visible in the diff).
> @@ -6090,7 +6105,31 @@
> == c_common_signed_type (mvr))
> && TYPE_ATOMIC (mvl) == TYPE_ATOMIC (mvr)))
> {
> - if (pedantic
> + /* Warn about conversions for pointers to arrays with different
> + qualifiers on the element type. Otherwise we only warn about
> + these as being incompatible pointers with -pedantic. */
> + if (OPT_Wdiscarded_array_qualifiers
> + && ((TREE_CODE (ttr) == ARRAY_TYPE)
> + || TREE_CODE (ttl) == ARRAY_TYPE))
> + {
> + ttr = strip_array_types(ttr);
Note there should be a space before the open parenthesis.
> + ttl = strip_array_types(ttl);
> +
> + if (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC (ttr)
> + & ~TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC (ttl))
> + WARN_FOR_QUALIFIERS (location, expr_loc,
WARN_FOR_QUALIFIERS uses pedwarn. That means this is not safe, because
pedwarns become errors with -pedantic-errors, but this includes cases that
are valid in ISO C and so must not become errors with -pedantic-errors
(such as converting const int (*)[] to void *).
So you need a variant of WARN_FOR_QUALIFIERS that uses warning_at instead
of pedwarn. But then you *also* need to be careful not to lose errors
with -pedantic-errors for cases where they are required by ISO C but not
with the C++ handling of qualifiers (such as converting const void * to
const int (*)[]) - so you can't have an if / else chain where an earlier
case gives a plain warning and stops a later case from running that would
give a pedwarn required by ISO C.
I think the correct logic might be something like:
* If the existing check for discarding qualifiers applies, then: recheck
the qualifiers after strip_array_types; give the existing
WARN_FOR_QUALIFIERS diagnostic if either qualifiers are still being
discarded with the C++-style interpretation, or -pedantic.
* As the next case after that existing check, see if qualifiers are being
discarded with the C++-style interpretation even though they weren't with
the C standard interpretation, and if so then give diagnostics using the
new macro that uses warning_at instead of pedwarn.
(And otherwise the code would fall through to the existing cases relating
to mismatch in signedness between two pointers to integers.)
--
Joseph S. Myers
joseph@codesourcery.com