This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] warning about const multidimensional array as function parameter


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]