On Sat, Sep 07, 2024 at 10:17:14PM +0100, Peter Frost wrote:
> v2 Patch:
>       * adds proper changelog text
>       * fixes typo in c.opt
> 
> Currently the behaviour of Wmissing-field-initializers is inconsistent
> between C and C++. The C warning assumes that missing designated 
> initializers are deliberate, and does not warn. The C++ warning does warn
> for missing designated initializers.
> 
> This patch changes the behaviour of Wmissing-field-initializers to
> universally not warn about missing designated initializers, and adds a new
> warning for specifically for missing designated initializers.
> 
> NOTE TO MAINTAINERS: This is my first gcc contribution, so I don't have
> git write access.

Thanks for the patch and sorry for the delay.

New options need to be documented in doc/invoke.texi; you can follow
-Wmissing-field-initializers in this case.
 
> Successfully tested on x86_64-pc-linux-gnu.
> 
>       PR c/39589
> 
> gcc/c-family/ChangeLog:
> 
>       * c.opt: Added new option Wmissing-designated-initializers, enabled by
>       Wextra

Usually we'd say:

        * c.opt (Wmissing-designated-initializers): New option.

> gcc/c/ChangeLog:
> 
>       * c-typeck.cc (pop_init_level): Generate warning for missing designated
>       initializers rather than always ignore

Please add a '.' at the end.

> gcc/cp/ChangeLog:
> 
>       * typeck2.cc (process_init_constructor_record): Add check if missing
>       initializer is designated, warn as appropriate

Here too.

> gcc/testsuite/ChangeLog:
> 
>       * g++.dg/diagnostic/base.C: Change flags
>       * gcc.dg/20011021-1.c: Fix test, missing designated initializers can
>       generate a warning now
>       * gcc.dg/missing-field-init-1.c: Change flags
>       * gcc.dg/pr60784.c: Change flags

Here too.

>       * g++.dg/warn/missing-designated-initializers-1.C: New test.
>       * g++.dg/warn/missing-designated-initializers-2.C: New test.
>       * gcc.dg/missing-designated-initializers-1.c: New test.
>       * gcc.dg/missing-designated-initializers-2.c: New test.
> 
> 
> ---
>  gcc/c-family/c.opt                            |  4 +++
>  gcc/c/c-typeck.cc                             | 36 +++++++++++--------
>  gcc/cp/typeck2.cc                             | 20 ++++++++---
>  gcc/testsuite/g++.dg/diagnostic/base.C        |  4 +--
>  .../warn/missing-designated-initializers-1.C  | 11 ++++++
>  .../warn/missing-designated-initializers-2.C  | 11 ++++++
>  gcc/testsuite/gcc.dg/20011021-1.c             |  4 +--
>  .../missing-designated-initializers-1.c       | 13 +++++++
>  .../missing-designated-initializers-2.c       | 13 +++++++
>  gcc/testsuite/gcc.dg/missing-field-init-1.c   |  2 +-
>  gcc/testsuite/gcc.dg/pr60784.c                |  2 +-
>  11 files changed, 96 insertions(+), 24 deletions(-)
>  create mode 100644 
> gcc/testsuite/g++.dg/warn/missing-designated-initializers-1.C
>  create mode 100644 
> gcc/testsuite/g++.dg/warn/missing-designated-initializers-2.C
>  create mode 100644 gcc/testsuite/gcc.dg/missing-designated-initializers-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/missing-designated-initializers-2.c
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 491aa02e1a3..81e52f1417e 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -977,6 +977,10 @@ Wmissing-field-initializers
>  C ObjC C++ ObjC++ Var(warn_missing_field_initializers) Warning 
> EnabledBy(Wextra)
>  Warn about missing fields in struct initializers.
>  
> +Wmissing-designated-initializers
> +C ObjC C++ ObjC++ Var(warn_missing_designated_initializers) Warning 
> EnabledBy(Wextra)
> +Warn about missing designated initializers in struct initializers.
> +

Since you're adding a new option, c.opt.urls needs to be regenerated.  All you
should need for that is to:
$ make html
$ make regenerate-opt-urls
in the builddir.  And I think you need to enable all languages when configuring
GCC for that to work.  Let me know if you're having trouble with this.

>  Wmissing-format-attribute
>  C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format)
>  ;
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 094e41fa202..72b544e8f67 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -9795,7 +9795,7 @@ pop_init_level (location_t loc, int implicit,
>      }
>  
>    /* Warn when some struct elements are implicitly initialized to zero.  */
> -  if (warn_missing_field_initializers
> +  if ((warn_missing_field_initializers || 
> warn_missing_designated_initializers)
>        && constructor_type
>        && TREE_CODE (constructor_type) == RECORD_TYPE
>        && constructor_unfilled_fields)
> @@ -9806,21 +9806,29 @@ pop_init_level (location_t loc, int implicit,
>                  || integer_zerop (DECL_SIZE (constructor_unfilled_fields))))
>         constructor_unfilled_fields = DECL_CHAIN 
> (constructor_unfilled_fields);
>  
> -     if (constructor_unfilled_fields
> -         /* Do not warn if this level of the initializer uses member
> -            designators; it is likely to be deliberate.  */
> -         && !constructor_designated
> -         /* Do not warn about initializing with { 0 } or with { }.  */
> -         && !constructor_zeroinit)
> -       {
> -         if (warning_at (input_location, OPT_Wmissing_field_initializers,
> +       if (constructor_unfilled_fields
> +           /* Do not warn about initializing with { 0 } or with { }.  */
> +           && !constructor_zeroinit)
> +         {
> +             if (!constructor_designated)
> +               {
> +                 if (warning_at (input_location, 
> OPT_Wmissing_field_initializers,
>                           "missing initializer for field %qD of %qT",
> -                         constructor_unfilled_fields,
> -                         constructor_type))
> -           inform (DECL_SOURCE_LOCATION (constructor_unfilled_fields),
> +                         constructor_unfilled_fields, constructor_type))
> +                       inform (DECL_SOURCE_LOCATION 
> (constructor_unfilled_fields),
> +                       "%qD declared here", constructor_unfilled_fields);
> +               }
> +             else if (warn_missing_designated_initializers)

I think this could be just "else".

> +               {
> +                 if (warning_at (
> +                         input_location, 
> OPT_Wmissing_designated_initializers,
> +                         "missing designated initializer for field %qD of 
> %qT",
> +                         constructor_unfilled_fields, constructor_type))
> +                       inform (DECL_SOURCE_LOCATION 
> (constructor_unfilled_fields),

The formatting should be more like:

  if (warning_at (input_location, OPT_Wmissing_designated_initializers,
                  "..."))
    inform (...);

>                     "%qD declared here", constructor_unfilled_fields);
> -       }
> -    }
> +               }
> +             }
> +     }
>  
>    /* Pad out the end of the structure.  */
>    if (p->replacement_value.value)
> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> index 30a6fbe95c9..6d21caf53d7 100644
> --- a/gcc/cp/typeck2.cc
> +++ b/gcc/cp/typeck2.cc
> @@ -1822,8 +1822,14 @@ process_init_constructor_record (tree type, tree init, 
> int nested, int flags,
>         if ((complain & tf_warning)
>             && !cp_unevaluated_operand
>             && !EMPTY_CONSTRUCTOR_P (init))
> -         warning (OPT_Wmissing_field_initializers,
> -                  "missing initializer for member %qD", field);
> +         {
> +           if (CONSTRUCTOR_IS_DESIGNATED_INIT (init))
> +                 warning (OPT_Wmissing_designated_initializers,
> +                 "missing designated initializer for member %qD", field);
> +           else
> +                 warning (OPT_Wmissing_field_initializers,
> +                 "missing initializer for member %qD", field);
> +         }

Formatting: the warning calls seem to be indented incorrectly.
The convention is:

  if (foo)
    warning (...,
             "foo");
  else
    warning (...,
             "bar");

>       }
>        else
>       {
> @@ -1853,8 +1859,14 @@ process_init_constructor_record (tree type, tree init, 
> int nested, int flags,
>             && !cp_unevaluated_operand
>             && !EMPTY_CONSTRUCTOR_P (init)
>             && !is_really_empty_class (fldtype, /*ignore_vptr*/false))
> -         warning (OPT_Wmissing_field_initializers,
> -                  "missing initializer for member %qD", field);
> +         {
> +           if (CONSTRUCTOR_IS_DESIGNATED_INIT (init))
> +                 warning (OPT_Wmissing_designated_initializers,
> +                   "missing designated initializer for member %qD", field);
> +           else
> +                 warning (OPT_Wmissing_field_initializers,
> +                   "missing initializer for member %qD", field);
> +         }

Here too.

>  
>         if (!zero_init_p (fldtype) || skipped < 0)
>           {
> diff --git a/gcc/testsuite/g++.dg/diagnostic/base.C 
> b/gcc/testsuite/g++.dg/diagnostic/base.C
> index 1540414072e..d8c644f78f5 100644
> --- a/gcc/testsuite/g++.dg/diagnostic/base.C
> +++ b/gcc/testsuite/g++.dg/diagnostic/base.C
> @@ -1,6 +1,6 @@
>  // PR c++/110745
>  // { dg-do compile { target c++17 } }
> -// { dg-options "-Wmissing-field-initializers" }
> +// { dg-options "-Wmissing-designated-initializers" }
>  
>  struct B { int i; };
>  struct D : B {
> @@ -11,6 +11,6 @@ struct D : B {
>  int
>  main ()
>  {
> -  D d = {.x=1, .y=2}; // { dg-warning "missing initializer for member 
> .D::B." }
> +  D d = {.x=1, .y=2}; // { dg-warning "missing designated initializer for 
> member .D::B." }
>    (void)d;
>  }
> diff --git a/gcc/testsuite/g++.dg/warn/missing-designated-initializers-1.C 
> b/gcc/testsuite/g++.dg/warn/missing-designated-initializers-1.C
> new file mode 100644
> index 00000000000..86afafc5e9b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/missing-designated-initializers-1.C
> @@ -0,0 +1,11 @@

Please add

  /* PR c/39589 */

to the new tests.

> +/* { dg-do compile } */
> +/* { dg-options "-Wextra -Wmissing-designated-initializers" } */
> +
> +struct s { int a, b, c; };
> +struct s s1 = { .a = 1, .b = 2, .c = 3 };
> +struct s s2 = { .a = 1, .b = 2 }; /* { dg-warning "missing designated 
> initializer" } */
> +struct s s3 = { .a = 1 }; /* { dg-warning "missing designated initializer" } 
> */
> +
> +struct s s6[] = { { .a = 1, .b = 2, .c = 3 }, { .a = 4 } }; /* { dg-warning 
> "missing designated initializer" } */
> +struct s s7[] = { { .a = 1 }, { .a = 2, .b = 3, .c = 4 } }; /* { dg-warning 
> "missing designated initializer" } */
> +struct s s8[] = { { 1, 2, 3 }, { .a = 4 } }; /* { dg-warning "missing 
> designated initializer" } */
> diff --git a/gcc/testsuite/g++.dg/warn/missing-designated-initializers-2.C 
> b/gcc/testsuite/g++.dg/warn/missing-designated-initializers-2.C
> new file mode 100644
> index 00000000000..b47af7baac5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/missing-designated-initializers-2.C
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wextra -Wno-missing-designated-initializers" } */
> +
> +struct s { int a, b, c; };
> +struct s s1 = { .a = 1, .b = 2, .c = 3 };
> +struct s s2 = { .a = 1, .b = 2 }; /* { dg-bogus "missing designated 
> initializer" } */
> +struct s s3 = { .a = 1 }; /* { dg-bogus "missing designated initializer" } */
> +
> +struct s s6[] = { { .a = 1, .b = 2, .c = 3 }, { .a = 4 } }; /* { dg-bogus 
> "missing designated initializer" } */
> +struct s s7[] = { { .a = 1 }, { .a = 2, .b = 3, .c = 4 } }; /* { dg-bogus 
> "missing designated initializer" } */
> +struct s s8[] = { { 1, 2, 3 }, { .a = 4 } }; /* { dg-bogus "missing 
> designated initializer" } */
> diff --git a/gcc/testsuite/gcc.dg/20011021-1.c 
> b/gcc/testsuite/gcc.dg/20011021-1.c
> index 2d567a75381..66964541d05 100644
> --- a/gcc/testsuite/gcc.dg/20011021-1.c
> +++ b/gcc/testsuite/gcc.dg/20011021-1.c
> @@ -29,7 +29,7 @@ struct multilevel
>  
>  struct t T0 = { 1 };         /* { dg-warning "missing init" } */
>  
> -struct t T1 = { .a = 1 };    /* { dg-bogus "(missing|near) init" } */
> +struct t T1 = { .a = 1 };    /* { dg-warning "missing designated 
> initializer" } */
>  
>  union u U0 = { 1 };          /* { dg-warning "initialization of union" } */
>  union u U1 = { .i = 1 };     /* { dg-bogus "initialization of union" } */
> @@ -37,7 +37,7 @@ union u U1 = { .i = 1 };    /* { dg-bogus "initialization 
> of union" } */
>  struct multilevel M =
>  {
>    12,
> -  { .b = 3 },                        /* { dg-bogus "missing init" } */
> +  { .b = 3 },                        /* { dg-warning "missing designated 
> initializer"  } */
>    { 4 },                     /* { dg-warning "initialization of union" } */
>    { .n = 9 },                        /* { dg-bogus "initialization of union" 
> } */
>    /* "string here" */
> diff --git a/gcc/testsuite/gcc.dg/missing-designated-initializers-1.c 
> b/gcc/testsuite/gcc.dg/missing-designated-initializers-1.c
> new file mode 100644
> index 00000000000..817386785f3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/missing-designated-initializers-1.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wextra -Wmissing-designated-initializers" } */
> +
> +struct s { int a, b, c; };
> +struct s s1 = { .a = 1, .b = 2, .c = 3 };
> +struct s s2 = { .a = 1, .b = 2 }; /* { dg-warning "missing designated 
> initializer" } */
> +struct s s3 = { .a = 1 }; /* { dg-warning "missing designated initializer" } 
> */
> +struct s s4 = { .a = 1, 2 }; /* { dg-warning "missing designated 
> initializer" } */
> +struct s s5 = { .a = 1, 2, 3 }; /* { dg-bogus "missing designated 
> initializer" } */
> +
> +struct s s6[] = { { .a = 1, .b = 2, .c = 3 }, { .a = 4 } }; /* { dg-warning 
> "missing designated initializer" } */
> +struct s s7[] = { { .a = 1 }, { .a = 2, .b = 3, .c = 4 } }; /* { dg-warning 
> "missing designated initializer" } */
> +struct s s8[] = { { 1, 2, 3 }, { .a = 4 } }; /* { dg-warning "missing 
> designated initializer" } */
> diff --git a/gcc/testsuite/gcc.dg/missing-designated-initializers-2.c 
> b/gcc/testsuite/gcc.dg/missing-designated-initializers-2.c
> new file mode 100644
> index 00000000000..dc2c6bb51f3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/missing-designated-initializers-2.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wextra -Wno-missing-designated-initializers" } */
> +
> +struct s { int a, b, c; };
> +struct s s1 = { .a = 1, .b = 2, .c = 3 };
> +struct s s2 = { .a = 1, .b = 2 }; /* { dg-bogus "missing designated 
> initializer" } */
> +struct s s3 = { .a = 1 }; /* { dg-bogus "missing designated initializer" } */
> +struct s s4 = { .a = 1, 2 }; /* { dg-bogus "missing designated initializer" 
> } */
> +struct s s5 = { .a = 1, 2, 3 }; /* { dg-bogus "missing designated 
> initializer" } */
> +
> +struct s s6[] = { { .a = 1, .b = 2, .c = 3 }, { .a = 4 } }; /* { dg-bogus 
> "missing designated initializer" } */
> +struct s s7[] = { { .a = 1 }, { .a = 2, .b = 3, .c = 4 } }; /* { dg-bogus 
> "missing designated initializer" } */
> +struct s s8[] = { { 1, 2, 3 }, { .a = 4 } }; /* { dg-bogus "missing 
> designated initializer" } */
> diff --git a/gcc/testsuite/gcc.dg/missing-field-init-1.c 
> b/gcc/testsuite/gcc.dg/missing-field-init-1.c
> index 5f32b021f0a..cd43fcc4f06 100644
> --- a/gcc/testsuite/gcc.dg/missing-field-init-1.c
> +++ b/gcc/testsuite/gcc.dg/missing-field-init-1.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-Wextra -Wno-missing-field-initializers -std=c99" } */
> +/* { dg-options "-Wextra -Wno-missing-field-initializers 
> -Wno-missing-designated-initializers -std=c99" } */
>  
>  struct s { int a, b, c; };
>  struct s s1 = { 1, 2, 3 };
> diff --git a/gcc/testsuite/gcc.dg/pr60784.c b/gcc/testsuite/gcc.dg/pr60784.c
> index 82b512f7c2e..60532831f4f 100644
> --- a/gcc/testsuite/gcc.dg/pr60784.c
> +++ b/gcc/testsuite/gcc.dg/pr60784.c
> @@ -1,6 +1,6 @@
>  /* PR c/60784 */
>  /* { dg-do compile } */
> -/* { dg-options "-Wextra -std=c99" } */
> +/* { dg-options "-Wextra -Wno-missing-designated-initializers -std=c99" } */
>  
>  struct A { int i, j; };
>  struct B { struct A a; } b1 = { .a.i = 1, .a.j = 1 };
> -- 
> 2.39.2
> 

Marek

Reply via email to