This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v4] warning about const multidimensional array as function parameter
- From: Joseph 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, 3 Dec 2014 22:33:06 +0000
- Subject: Re: [PATCH v4] 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> <Pine dot LNX dot 4 dot 64 dot 1410291623450 dot 21975 at digraph dot polyomino dot org dot uk> <20141106101820 dot 22ee3429 at lemur> <alpine dot DEB dot 2 dot 10 dot 1411062339490 dot 17254 at digraph dot polyomino dot org dot uk> <20141110110805 dot 576e37ee at lemur>
On Mon, 10 Nov 2014, Martin Uecker wrote:
> + if (OPT_Wdiscarded_array_qualifiers
OPT_Wdiscarded_array_qualifiers is an enum constant, so it doesn't make
sense to test it in if conditions like this. You don't need a test in the
if condition for whether the warning is enabled - passing
OPT_Wdiscarded_array_qualifiers to warning_at is sufficient to cause the
warning to appear only if enabled - but if you consider the rest of the
test for whether to warn to be expensive so you don't want to execute it
unless this warning is enabled, you can test the variable (actually a
macro for a structure field) warn_discarded_array_qualifiers.
> + && (TREE_CODE (TREE_TYPE (type2)) == ARRAY_TYPE)
> + && (TYPE_QUALS (strip_array_types (TREE_TYPE (type2)))
> + & ~TYPE_QUALS (TREE_TYPE (type1))))
> + warning_at(colon_loc, OPT_Wdiscarded_array_qualifiers,
> + "pointer to array loses qualifier "
> + "in conditional expression");
Missing space before '(' in call to warning_at.
> @@ -4613,6 +4636,14 @@ build_conditional_expr (location_t colon_loc, tree
> else if (VOID_TYPE_P (TREE_TYPE (type2))
> && !TYPE_ATOMIC (TREE_TYPE (type2)))
> {
> + if (OPT_Wdiscarded_array_qualifiers
> + && (TREE_CODE (TREE_TYPE (type1)) == ARRAY_TYPE)
> + && (TYPE_QUALS (strip_array_types (TREE_TYPE (type1)))
> + & ~TYPE_QUALS (TREE_TYPE (type2))))
> + warning_at(colon_loc, OPT_Wdiscarded_array_qualifiers,
> + "pointer to array loses qualifier "
> + "in conditional expression");
Same issues here.
> @@ -6090,42 +6149,70 @@ convert_for_assignment (location_t location, locat
> == c_common_signed_type (mvr))
> && TYPE_ATOMIC (mvl) == TYPE_ATOMIC (mvr)))
> {
> - if (pedantic
> + /* Warn about loss of qualifers from pointers to arrays with
> + qualifiers on the element type. */
> + if (OPT_Wdiscarded_array_qualifiers
Another spurious test of OPT_Wdiscarded_array_qualifiers.
> /* Const and volatile mean something different for function types,
> so the usual warnings are not appropriate. */
> else if (TREE_CODE (ttr) != FUNCTION_TYPE
> && TREE_CODE (ttl) != FUNCTION_TYPE)
> {
> + /* Don't warn about loss of qualifier for conversions from
> + qualified void* to pointers to arrays with corresponding
> + qualifier on the the element type. */
> + if (!pedantic)
> + ttl = strip_array_types (ttl);
> +
> /* Assignments between atomic and non-atomic objects are OK. */
> - if (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC (ttr)
> + if (TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC (ttr)
> & ~TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC (ttl))
There seems to be something wrong with the indentation of the new code
here, and as far as I can tell the indentation of the line you reindented
was correct before the patch.
I didn't spot any substantive problems in this round of review, so suspect
the next revision of the patch may be good to go in.
--
Joseph S. Myers
joseph@codesourcery.com