On Wed, Jun 04, 2014 at 03:15:02PM +0200, Marek Polacek wrote:
> On Mon, Jun 02, 2014 at 12:58:53AM -0400, S. Gilles wrote:
>
> Thanks for tackling this.
>
> > @@ -6858,6 +6858,9 @@
> > /* 1 if this constructor is erroneous so far. */
> > static int constructor_erroneous;
> >
> > +/* 1 if this constructor is the universal zero initializer { 0 } */
>
> ". */" at the end of the comment, please.
Applied.
>
> > @@ -7317,12 +7321,8 @@
> > set_nonincremental_init (braced_init_obstack);
> > }
> >
> > - if (implicit == 1 && warn_missing_braces && !missing_braces_mentioned)
> > - {
> > - missing_braces_mentioned = 1;
> > - warning_init (input_location, OPT_Wmissing_braces,
> > - "missing braces around initializer");
> > - }
> > + if (implicit == 1)
> > + found_missing_braces = 1;
>
> Wrong indentation, there should be only two spaces.
Applied.
>
> > + /* Warn when some structs are initialized with direct aggregation. */
> > + if (!implicit && found_missing_braces && warn_missing_braces
> > + && !constructor_zeroinit)
> > + {
> > + warning_init (input_location, OPT_Wmissing_braces,
> > + "missing braces around initializer");
> > + }
> > +
>
> Please use "loc" instead of input_location.
Applied.
>
> > @@ -8594,6 +8598,11 @@
> > designator_depth = 0;
> > designator_erroneous = 0;
> >
> > + if (!implicit && value.value && !integer_zerop (value.value))
> > + {
> > + constructor_zeroinit = 0;
> > + }
> > +
>
> You can drop the braces here.
Applied - thanks for catching these.
>
> But the problem I see is that this patch treats even "{ 0, 0 }" as
> a "universal zero initializer", thus we wouldn't warn on say:
>
> /* -Wmissing-braces -Wmissing-field-initializers */
> struct T { int a, b; };
> struct U { int a, b; struct T t; };
>
> void
> f (void)
> {
> int a[2][2][2] = { 0, 0, 0 };
> struct U u = { 0, 0 };
> }
>
> I'm not sure whether that is desirable.
That is/was partially intentional. I probably should have mentioned that
originally, especially with the { 0, 0 } in the test case.
I believe an argument can be made that this behavior isn't undesirable:
the standard (unless I am mistaken) makes initialization to any number
of zeros equivalent (as long as there are not more zeros than elements),
so anyone who initializes a struct to { 0, 0 } is probably trying to
zero it out. If they don't use braces when they should, or if they use
six zeros instead of seven, the error will result in the structure being
initialized as they wanted anyway. If they use nonzero values for part
of their initialization, then there's a chance that the definition has
changed, that they aren't intentionally applying aggregation, etc.
As I write it, though, that argument seems far less compelling than when
I thought of it, especially since, with this patch, an assignment of
something like { { 0, 0 }, 0 } would cause warnings, and the `it still
probably works as they intended' argument would still apply. It makes
much more sense to have { 0 } as the only exception to properly filling
out an assignment.
Therefore, I have also amended the patch to add the length-checking from
the previous logic. The test case is also updated to make sure that
holds. The amended patch is attached (along with the ChangeLog entries
from the first version).
>
> Marek
--
S. Gilles
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c (revision 211081)
+++ gcc/c/c-typeck.c (working copy)
@@ -74,9 +74,9 @@
if expr.original_code == SIZEOF_EXPR. */
tree c_last_sizeof_arg;
-/* Nonzero if we've already printed a "missing braces around initializer"
- message within this initializer. */
-static int missing_braces_mentioned;
+/* Nonzero if we might need to print a "missing braces around
+ initializer" message within this initializer. */
+static int found_missing_braces;
static int require_constant_value;
static int require_constant_elements;
@@ -6858,6 +6858,9 @@
/* 1 if this constructor is erroneous so far. */
static int constructor_erroneous;
+/* 1 if this constructor is the universal zero initializer { 0 }. */
+static int constructor_zeroinit;
+
/* Structure for managing pending initializer elements, organized as an
AVL tree. */
@@ -7019,7 +7022,7 @@
constructor_stack = 0;
constructor_range_stack = 0;
- missing_braces_mentioned = 0;
+ found_missing_braces = 0;
spelling_base = 0;
spelling_size = 0;
@@ -7114,6 +7117,7 @@
constructor_type = type;
constructor_incremental = 1;
constructor_designated = 0;
+ constructor_zeroinit = 1;
designator_depth = 0;
designator_erroneous = 0;
@@ -7317,12 +7321,8 @@
set_nonincremental_init (braced_init_obstack);
}
- if (implicit == 1 && warn_missing_braces && !missing_braces_mentioned)
- {
- missing_braces_mentioned = 1;
- warning_init (input_location, OPT_Wmissing_braces,
- "missing braces around initializer");
- }
+ if (implicit == 1)
+ found_missing_braces = 1;
if (TREE_CODE (constructor_type) == RECORD_TYPE
|| TREE_CODE (constructor_type) == UNION_TYPE)
@@ -7455,6 +7455,17 @@
}
}
+ if (vec_safe_length (constructor_elements) != 1)
+ constructor_zeroinit = 0;
+
+ /* Warn when some structs are initialized with direct aggregation. */
+ if (!implicit && found_missing_braces && warn_missing_braces
+ && !constructor_zeroinit)
+ {
+ warning_init (loc, OPT_Wmissing_braces,
+ "missing braces around initializer");
+ }
+
/* Warn when some struct elements are implicitly initialized to zero. */
if (warn_missing_field_initializers
&& constructor_type
@@ -7461,10 +7472,6 @@
&& TREE_CODE (constructor_type) == RECORD_TYPE
&& constructor_unfilled_fields)
{
- bool constructor_zeroinit =
- (vec_safe_length (constructor_elements) == 1
- && integer_zerop ((*constructor_elements)[0].value));
-
/* Do not warn for flexible array members or zero-length arrays. */
while (constructor_unfilled_fields
&& (!DECL_SIZE (constructor_unfilled_fields)
@@ -8594,6 +8601,9 @@
designator_depth = 0;
designator_erroneous = 0;
+ if (!implicit && value.value && !integer_zerop (value.value))
+ constructor_zeroinit = 0;
+
/* Handle superfluous braces around string cst as in
char x[] = {"foo"}; */
if (string_flag
Index: gcc/testsuite/gcc.dg/pr53119.c
===================================================================
--- gcc/testsuite/gcc.dg/pr53119.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr53119.c (working copy)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+
+/* { dg-options "-Wmissing-braces -Wmissing-field-initializers" } */
+
+struct a {
+ int x, y, z;
+};
+
+struct b {
+ struct a w, z;
+};
+
+int main (void)
+{
+ struct a az = { 0 };
+ struct a anz = { 1 }; /* { dg-warning "missing initializer for" } */
+ struct a aez = { 0, 0 }; /* { dg-warning "missing initializer for" } */
+
+ struct b bz = { 0 };
+ struct b bnz = { 0, 0, 0, 0, 0, 0 }; /* { dg-warning "missing braces" } */
+
+ return 0;
+}
+
+/* { dg-excess-errors "note" } */
gcc/ChangeLog:
PR c/53119
c/
* c-typeck.c (push_init_level, process_init_element,
pop_init_level): Correct check for zero initialization, move
missing brace warning to respect zero initialization.
* gcc.dg/pr53119.c: New testcase.
