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
[email protected]
