public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
@ 2020-06-03 22:51 Asher Gordon
  2020-06-03 23:03 ` Asher Gordon
  2020-06-07 17:50 ` [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 } Asher Gordon
  0 siblings, 2 replies; 29+ messages in thread
From: Asher Gordon @ 2020-06-03 22:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: Asher Gordon

Closes #95379.

gcc/ChangeLog:

	* doc/extend.texi: Document { 0 } as a special case for the
	designated_init attribute.

gcc/c/ChangeLog:

	* c-typeck.c (struct location_list): New type.
	(struct initializer_stack): Add positional_init_locs for
	-Wdesignated-init warnings.
	(start_init): Initialize initializer_stack->positional_init_locs
	to NULL.
	(finish_init): Free initializer_stack->positional_init_locs.
	(pop_init_level): Move -Wdesignated-init warning here from
	process_init_element so that we can treat { 0 } specially.
	(process_init_element): Instead of warning on -Wdesignated-init
	here, remember a list of locations where we should warn and do the
	actual warning in pop_init_level.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.
---
Note: Please see the discussion here[1] before applying this
patch. Also note that this patch does not implement a
-Wno-designated-init flag, but that shouldn't be too hard to
implement.

[1]  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95379

 gcc/ChangeLog                           |  5 +++
 gcc/c/ChangeLog                         | 14 +++++++
 gcc/c/c-typeck.c                        | 55 +++++++++++++++++++++++--
 gcc/doc/extend.texi                     |  4 ++
 gcc/testsuite/ChangeLog                 |  4 ++
 gcc/testsuite/gcc.dg/Wdesignated-init.c |  3 ++
 6 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3fc8a8e55cc..0249f965c4f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* doc/extend.texi: Document { 0 } as a special case for the
+	designated_init attribute.
+
 2020-06-02  Felix Yang  <felix.yang@huawei.com>
 
 	PR target/95459
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 7efb6bfc544..0a6cf36bad7 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,17 @@
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c-typeck.c (struct location_list): New type.
+	(struct initializer_stack): Add positional_init_locs for
+	-Wdesignated-init warnings.
+	(start_init): Initialize initializer_stack->positional_init_locs
+	to NULL.
+	(finish_init): Free initializer_stack->positional_init_locs.
+	(pop_init_level): Move -Wdesignated-init warning here from
+	process_init_element so that we can treat { 0 } specially.
+	(process_init_element): Instead of warning on -Wdesignated-init
+	here, remember a list of locations where we should warn and do the
+	actual warning in pop_init_level.
+
 2020-05-28  Nicolas Bértolo  <nicolasbertolo@gmail.com>
 
 	* Make-lang.in: Remove extra slash.
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 385bf3a1c7b..6273a7ce2d7 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8179,6 +8179,13 @@ struct constructor_range_stack
 
 static struct constructor_range_stack *constructor_range_stack;
 
+/* A list of locations.  */
+
+struct location_list {
+  struct location_list *next;
+  location_t loc;
+};
+
 /* This stack records separate initializers that are nested.
    Nested initializers can't happen in ANSI C, but GNU C allows them
    in cases like { ... (struct foo) { ... } ... }.  */
@@ -8197,6 +8204,7 @@ struct initializer_stack
   char require_constant_value;
   char require_constant_elements;
   rich_location *missing_brace_richloc;
+  struct location_list *positional_init_locs;
 };
 
 static struct initializer_stack *initializer_stack;
@@ -8222,6 +8230,7 @@ start_init (tree decl, tree asmspec_tree ATTRIBUTE_UNUSED, int top_level,
   p->top_level = constructor_top_level;
   p->next = initializer_stack;
   p->missing_brace_richloc = richloc;
+  p->positional_init_locs = NULL;
   initializer_stack = p;
 
   constructor_decl = decl;
@@ -8287,6 +8296,13 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
+
+  while (p->positional_init_locs)
+    {
+      struct location_list *list = p->positional_init_locs;
+      p->positional_init_locs = list->next;
+      free (list);
+    }
   free (p);
 }
 \f
@@ -8744,6 +8760,22 @@ pop_init_level (location_t loc, int implicit,
 	  }
     }
 
+  /* Warn when positional initializers are used for a structure with
+     the designated_init attribute, but make an exception for { 0 }.  */
+  while (initializer_stack->positional_init_locs)
+    {
+      struct location_list *loc = initializer_stack->positional_init_locs;
+
+      if (!constructor_zeroinit)
+	warning_init (loc->loc,
+		      OPT_Wdesignated_init,
+		      "positional initialization of field in %<struct%> "
+		      "declared with %<designated_init%> attribute");
+
+      initializer_stack->positional_init_locs = loc->next;
+      free(loc);
+    }
+
   /* Pad out the end of the structure.  */
   if (p->replacement_value.value)
     /* If this closes a superfluous brace pair,
@@ -9975,10 +10007,25 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
       && TREE_CODE (constructor_type) == RECORD_TYPE
       && lookup_attribute ("designated_init",
 			   TYPE_ATTRIBUTES (constructor_type)))
-    warning_init (loc,
-		  OPT_Wdesignated_init,
-		  "positional initialization of field "
-		  "in %<struct%> declared with %<designated_init%> attribute");
+    {
+      struct location_list *new_loc;
+
+      /* Remember where we got a positional initializer so we can warn
+	 if it initializes a field of a structure with the
+	 designated_init attribute.  */
+      new_loc = XNEW (struct location_list);
+      new_loc->next = NULL;
+      new_loc->loc = loc;
+      if (initializer_stack->positional_init_locs)
+	{
+	  struct location_list *last = initializer_stack->positional_init_locs;
+	  while (last->next)
+	    last++;
+	  last->next = new_loc;
+	}
+      else
+	initializer_stack->positional_init_locs = new_loc;
+    }
 
   /* If we've exhausted any levels that didn't have braces,
      pop them now.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index cced19d2018..a3c5059a8b0 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8093,6 +8093,10 @@ attribute is to allow the programmer to indicate that a structure's
 layout may change, and that therefore relying on positional
 initialization will result in future breakage.
 
+Note that the universal zero initializer, @code{@{ 0 @}}, is considered
+a special case and will not produce a warning when used to initialize a
+structure with the @code{designated_init} attribute.
+
 GCC emits warnings based on this attribute by default; use
 @option{-Wno-designated-init} to suppress them.
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 23f21cc8f4d..9c39195ba21 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.
+
 2020-06-02  David Malcolm  <dmalcolm@redhat.com>
 
 	PR jit/95426
diff --git a/gcc/testsuite/gcc.dg/Wdesignated-init.c b/gcc/testsuite/gcc.dg/Wdesignated-init.c
index b9ca572206c..adb7949df55 100644
--- a/gcc/testsuite/gcc.dg/Wdesignated-init.c
+++ b/gcc/testsuite/gcc.dg/Wdesignated-init.c
@@ -24,6 +24,9 @@ struct Des {
 struct Des d1 = { 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
 struct Des d2 = { .x = 5, .y = 5 };
 struct Des d3 = { .x = 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d4 = { 0 };
+struct Des d5 = { 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d6 = { 0, 0 }; /* { dg-warning "(positional|near initialization)" } */
 
 struct Des fd1 (void)
 {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-03 22:51 [PATCH] Treat { 0 } specially for structs with the designated_init attribute Asher Gordon
@ 2020-06-03 23:03 ` Asher Gordon
  2020-06-04  3:57   ` Asher Gordon
  2020-06-07 17:50 ` [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 } Asher Gordon
  1 sibling, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-03 23:03 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Asher Gordon <AsDaGo@posteo.net> writes:

> Also note that this patch does not implement a -Wno-designated-init
> flag, but that shouldn't be too hard to implement.

Sorry, I meant -Wno-universal-initializer, not -Wno-designated-init.

Asher

-- 
Vimes grunted.  "Where there are policemen there's crime, sergeant,
remember that."

"Yes, I do, sir, although I think it sounds better with a little reordering
of the words."

  [Snuff, by Terry Pratchett]
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-03 23:03 ` Asher Gordon
@ 2020-06-04  3:57   ` Asher Gordon
  2020-06-08 17:27     ` Martin Sebor
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-04  3:57 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 155 bytes --]

Hello,

I accidentally wrote 'free(loc)' instead of 'free (loc)'. Please see the
fixed patch attached below (contrib/check_GNU_style.sh says it's OK
now):

[-- Attachment #1.2: Fixed patch that conforms with the GNU coding standards --]
[-- Type: text/x-patch, Size: 8090 bytes --]

From 0445fba96ee9030feb00ebec893f8dfed153b12d Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Wed, 3 Jun 2020 17:20:08 -0400
Subject: [PATCH] Treat { 0 } specially for structs with the designated_init
 attribute.

Closes #95379.

gcc/ChangeLog:

	* doc/extend.texi: Document { 0 } as a special case for the
	designated_init attribute.

gcc/c/ChangeLog:

	* c-typeck.c (struct location_list): New type.
	(struct initializer_stack): Add positional_init_locs for
	-Wdesignated-init warnings.
	(start_init): Initialize initializer_stack->positional_init_locs
	to NULL.
	(finish_init): Free initializer_stack->positional_init_locs.
	(pop_init_level): Move -Wdesignated-init warning here from
	process_init_element so that we can treat { 0 } specially.
	(process_init_element): Instead of warning on -Wdesignated-init
	here, remember a list of locations where we should warn and do the
	actual warning in pop_init_level.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.
---
 gcc/ChangeLog                           |  5 +++
 gcc/c/ChangeLog                         | 14 +++++++
 gcc/c/c-typeck.c                        | 55 +++++++++++++++++++++++--
 gcc/doc/extend.texi                     |  4 ++
 gcc/testsuite/ChangeLog                 |  4 ++
 gcc/testsuite/gcc.dg/Wdesignated-init.c |  3 ++
 6 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index edbcaf2bc4d..fd60a248226 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -95,6 +95,11 @@
 	(lto_tree_code_to_tag): Update.
 	(lto_tag_to_tree_code): Update.
 
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* doc/extend.texi: Document { 0 } as a special case for the
+	designated_init attribute.
+
 2020-06-02  Felix Yang  <felix.yang@huawei.com>
 
 	PR target/95459
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index abf31e57688..e77f46930ef 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -17,6 +17,20 @@
 
 	* c-objc-common.h (LANG_HOOKS_OMP_PREDETERMINED_MAPPING): Redefine.
 
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c-typeck.c (struct location_list): New type.
+	(struct initializer_stack): Add positional_init_locs for
+	-Wdesignated-init warnings.
+	(start_init): Initialize initializer_stack->positional_init_locs
+	to NULL.
+	(finish_init): Free initializer_stack->positional_init_locs.
+	(pop_init_level): Move -Wdesignated-init warning here from
+	process_init_element so that we can treat { 0 } specially.
+	(process_init_element): Instead of warning on -Wdesignated-init
+	here, remember a list of locations where we should warn and do the
+	actual warning in pop_init_level.
+
 2020-05-28  Nicolas Bértolo  <nicolasbertolo@gmail.com>
 
 	* Make-lang.in: Remove extra slash.
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 385bf3a1c7b..2d04f70f0cf 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8179,6 +8179,13 @@ struct constructor_range_stack
 
 static struct constructor_range_stack *constructor_range_stack;
 
+/* A list of locations.  */
+
+struct location_list {
+  struct location_list *next;
+  location_t loc;
+};
+
 /* This stack records separate initializers that are nested.
    Nested initializers can't happen in ANSI C, but GNU C allows them
    in cases like { ... (struct foo) { ... } ... }.  */
@@ -8197,6 +8204,7 @@ struct initializer_stack
   char require_constant_value;
   char require_constant_elements;
   rich_location *missing_brace_richloc;
+  struct location_list *positional_init_locs;
 };
 
 static struct initializer_stack *initializer_stack;
@@ -8222,6 +8230,7 @@ start_init (tree decl, tree asmspec_tree ATTRIBUTE_UNUSED, int top_level,
   p->top_level = constructor_top_level;
   p->next = initializer_stack;
   p->missing_brace_richloc = richloc;
+  p->positional_init_locs = NULL;
   initializer_stack = p;
 
   constructor_decl = decl;
@@ -8287,6 +8296,13 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
+
+  while (p->positional_init_locs)
+    {
+      struct location_list *list = p->positional_init_locs;
+      p->positional_init_locs = list->next;
+      free (list);
+    }
   free (p);
 }
 \f
@@ -8744,6 +8760,22 @@ pop_init_level (location_t loc, int implicit,
 	  }
     }
 
+  /* Warn when positional initializers are used for a structure with
+     the designated_init attribute, but make an exception for { 0 }.  */
+  while (initializer_stack->positional_init_locs)
+    {
+      struct location_list *loc = initializer_stack->positional_init_locs;
+
+      if (!constructor_zeroinit)
+	warning_init (loc->loc,
+		      OPT_Wdesignated_init,
+		      "positional initialization of field in %<struct%> "
+		      "declared with %<designated_init%> attribute");
+
+      initializer_stack->positional_init_locs = loc->next;
+      free (loc);
+    }
+
   /* Pad out the end of the structure.  */
   if (p->replacement_value.value)
     /* If this closes a superfluous brace pair,
@@ -9975,10 +10007,25 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
       && TREE_CODE (constructor_type) == RECORD_TYPE
       && lookup_attribute ("designated_init",
 			   TYPE_ATTRIBUTES (constructor_type)))
-    warning_init (loc,
-		  OPT_Wdesignated_init,
-		  "positional initialization of field "
-		  "in %<struct%> declared with %<designated_init%> attribute");
+    {
+      struct location_list *new_loc;
+
+      /* Remember where we got a positional initializer so we can warn
+	 if it initializes a field of a structure with the
+	 designated_init attribute.  */
+      new_loc = XNEW (struct location_list);
+      new_loc->next = NULL;
+      new_loc->loc = loc;
+      if (initializer_stack->positional_init_locs)
+	{
+	  struct location_list *last = initializer_stack->positional_init_locs;
+	  while (last->next)
+	    last++;
+	  last->next = new_loc;
+	}
+      else
+	initializer_stack->positional_init_locs = new_loc;
+    }
 
   /* If we've exhausted any levels that didn't have braces,
      pop them now.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index cced19d2018..a3c5059a8b0 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8093,6 +8093,10 @@ attribute is to allow the programmer to indicate that a structure's
 layout may change, and that therefore relying on positional
 initialization will result in future breakage.
 
+Note that the universal zero initializer, @code{@{ 0 @}}, is considered
+a special case and will not produce a warning when used to initialize a
+structure with the @code{designated_init} attribute.
+
 GCC emits warnings based on this attribute by default; use
 @option{-Wno-designated-init} to suppress them.
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 89a1ad55923..a155c3c5332 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -67,6 +67,10 @@
 	PR middle-end/94874
 	* c-c++-common/gomp/pr94874.c: New.
 
+2020-06-03  Asher Gordon  <AsDaGo@posteo.net>
+
+	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.
+
 2020-06-02  David Malcolm  <dmalcolm@redhat.com>
 
 	PR jit/95426
diff --git a/gcc/testsuite/gcc.dg/Wdesignated-init.c b/gcc/testsuite/gcc.dg/Wdesignated-init.c
index b9ca572206c..adb7949df55 100644
--- a/gcc/testsuite/gcc.dg/Wdesignated-init.c
+++ b/gcc/testsuite/gcc.dg/Wdesignated-init.c
@@ -24,6 +24,9 @@ struct Des {
 struct Des d1 = { 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
 struct Des d2 = { .x = 5, .y = 5 };
 struct Des d3 = { .x = 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d4 = { 0 };
+struct Des d5 = { 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d6 = { 0, 0 }; /* { dg-warning "(positional|near initialization)" } */
 
 struct Des fd1 (void)
 {
-- 
2.26.2


[-- Attachment #1.3: Type: text/plain, Size: 398 bytes --]

Thanks,
Asher

-- 
<cas> well there ya go.  say something stupid in irc and have it
      immortalised forever in someone's .sig file
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 }.
  2020-06-03 22:51 [PATCH] Treat { 0 } specially for structs with the designated_init attribute Asher Gordon
  2020-06-03 23:03 ` Asher Gordon
@ 2020-06-07 17:50 ` Asher Gordon
  2020-06-07 20:40   ` Asher Gordon
  1 sibling, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-07 17:50 UTC (permalink / raw)
  To: gcc-patches; +Cc: Asher Gordon

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wuniversal-initializer.

gcc/c-family/ChangeLog:

	* c.opt: Add -Wuniversal-initializer

gcc/c/ChangeLog:

	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
	if warn_zero_init.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wuniversal-initializer.c: New test.
---
Asher Gordon <AsDaGo@posteo.net> writes:

> Also note that this patch does not implement a
> [-Wuniversal-initializer] flag, but that shouldn't be too hard to
> implement.

This patch now implements that.

 gcc/ChangeLog                                 |  4 ++++
 gcc/c-family/ChangeLog                        |  4 ++++
 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/ChangeLog                               |  5 +++++
 gcc/c/c-typeck.c                              |  9 ++++----
 gcc/doc/invoke.texi                           | 22 ++++++++++++++++++-
 gcc/testsuite/ChangeLog                       |  4 ++++
 gcc/testsuite/gcc.dg/Wuniversal-initializer.c | 20 +++++++++++++++++
 8 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wuniversal-initializer.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d569416c02f..15d520d3656 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* doc/invoke.texi: Document -Wuniversal-initializer.
+
 2020-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	PR lto/95548
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 8fac84d3b02..6824d7ea130 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c.opt: Add -Wuniversal-initializer
+
 2020-06-05  Jason Merrill  <jason@redhat.com>
 
 	* c-pretty-print.c (pp_c_additive_expression): Handle negative
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 89a58282b3f..15bbb8e69f7 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1191,6 +1191,10 @@ Wuninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
+Wuniversal-initializer
+C ObjC C++ ObjC++ Var(warn_zero_init) Init(1) Warning
+Don't suppress warnings about { 0 }.
+
 Wmaybe-uninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 50fb1e87ad6..3ade67830e8 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,8 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
+	if warn_zero_init.
+
 2020-06-05  Mark Wielaard  <mark@klomp.org>
 
 	* c-decl.c (implicit_decl_warning): When warned and olddecl is
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 2d04f70f0cf..e1071472301 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8724,7 +8724,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when some structs are initialized with direct aggregation.  */
   if (!implicit && found_missing_braces && warn_missing_braces
-      && !constructor_zeroinit)
+      && (!constructor_zeroinit || warn_zero_init))
     {
       gcc_assert (initializer_stack->missing_brace_richloc);
       warning_at (initializer_stack->missing_brace_richloc,
@@ -8748,8 +8748,9 @@ pop_init_level (location_t loc, int implicit,
 	    /* 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)
+	    /* Do not warn about initializing with { 0 } or with { }
+	       unless warn_zero_init.  */
+	    && (!constructor_zeroinit || warn_zero_init))
 	  {
 	    if (warning_at (input_location, OPT_Wmissing_field_initializers,
 			    "missing initializer for field %qD of %qT",
@@ -8766,7 +8767,7 @@ pop_init_level (location_t loc, int implicit,
     {
       struct location_list *loc = initializer_stack->positional_init_locs;
 
-      if (!constructor_zeroinit)
+      if (!constructor_zeroinit || warn_zero_init)
 	warning_init (loc->loc,
 		      OPT_Wdesignated_init,
 		      "positional initialization of field in %<struct%> "
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 06a04e3d7dd..f0893c250e7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -369,7 +369,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-switch-outside-range  -Wno-switch-unreachable  -Wsync-nand @gol
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
 -Wtype-limits  -Wundef @gol
--Wuninitialized  -Wunknown-pragmas @gol
+-Wuninitialized  -Wno-universal-initializer  -Wunknown-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused @gol
 -Wunused-but-set-parameter  -Wunused-but-set-variable @gol
 -Wunused-const-variable  -Wunused-const-variable=@var{n} @gol
@@ -6453,6 +6453,26 @@ to compute a value that itself is never used, because such
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
+@item -Wuniversal-initializer
+@opindex Wuniversal-initializer
+@opindex Wno-universal-initializer
+Do not suppress warnings about the universal zero initializer,
+@code{@{ 0 @}}, where a warning would otherwise be produced.  For
+example, even with @option{-Wmissing-braces}, the following would not
+warn, because an exception is made for the universal initializer:
+
+@smallexample
+int a[1][1] = @{ 0 @};
+@end smallexample
+
+However, if @code{-Wuniversal-initializer} is used, that code would
+produce a warning (caused by @option{-Wmissing-braces}).
+
+This warning is not enabled by default, nor is it enabled by any other
+options.  If you don't want to suppress warnings about the universal
+zero initializer, you must specify @option{-Wuniversal-initializer}
+explicitly.
+
 @item -Wno-invalid-memory-model
 @opindex Winvalid-memory-model
 @opindex Wno-invalid-memory-model
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 825f60b9d5b..65bd6fd6028 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* gcc.dg/Wuniversal-initializer.c: New test.
+
 2020-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	* g++.dg/torture/pr95548.C: New test.
diff --git a/gcc/testsuite/gcc.dg/Wuniversal-initializer.c b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
new file mode 100644
index 00000000000..803dfd10e1c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wmissing-field-initializers -Wuniversal-initializer" } */
+
+struct Pok {
+  int x;
+  int y;
+};
+
+struct Nest {
+  struct Pok p;
+};
+
+struct Des {
+  int a;
+} __attribute__((designated_init));
+
+struct Pok p = { 0 }; /* { dg-warning "missing initializer for field" } */
+struct Nest n = { 0 }; /* { dg-warning "missing braces around initializer" } */
+struct Des d = { 0 }; /* { dg-warning "(positional|near initialization)" } */
+int a[1][1] = { 0 };  /* { dg-warning "missing braces around initializer" } */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 }.
  2020-06-07 17:50 ` [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 } Asher Gordon
@ 2020-06-07 20:40   ` Asher Gordon
  2020-06-09 21:24     ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-07 20:40 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 101 bytes --]

Hello,

I accidentally used Init(1) for the option instead of Init(0). The
correction is as follows:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Change Init(1) to Init(0) --]
[-- Type: text/x-patch, Size: 441 bytes --]

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 15bbb8e69f7..8bfa28e5f6c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1192,7 +1192,7 @@ C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
 Wuniversal-initializer
-C ObjC C++ ObjC++ Var(warn_zero_init) Init(1) Warning
+C ObjC C++ ObjC++ Var(warn_zero_init) Init(0) Warning
 Don't suppress warnings about { 0 }.
 
 Wmaybe-uninitialized

[-- Attachment #1.3: Type: text/plain, Size: 32 bytes --]

Here is the entire fixed patch:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: The full patch with Init(0) --]
[-- Type: text/x-patch, Size: 7306 bytes --]

From 93a5f60bf72e37538cad3d4fb81da985b97fba8e Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Sun, 7 Jun 2020 13:37:27 -0400
Subject: [PATCH] Add -Wuniversal-initializer to not suppress warnings about {
 0 }.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wuniversal-initializer.

gcc/c-family/ChangeLog:

	* c.opt: Add -Wuniversal-initializer

gcc/c/ChangeLog:

	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
	if warn_zero_init.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wuniversal-initializer.c: New test.
---
 gcc/ChangeLog                                 |  4 ++++
 gcc/c-family/ChangeLog                        |  4 ++++
 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/ChangeLog                               |  5 +++++
 gcc/c/c-typeck.c                              |  9 ++++----
 gcc/doc/invoke.texi                           | 22 ++++++++++++++++++-
 gcc/testsuite/ChangeLog                       |  4 ++++
 gcc/testsuite/gcc.dg/Wuniversal-initializer.c | 20 +++++++++++++++++
 8 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wuniversal-initializer.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d569416c02f..15d520d3656 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* doc/invoke.texi: Document -Wuniversal-initializer.
+
 2020-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	PR lto/95548
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 8fac84d3b02..6824d7ea130 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c.opt: Add -Wuniversal-initializer
+
 2020-06-05  Jason Merrill  <jason@redhat.com>
 
 	* c-pretty-print.c (pp_c_additive_expression): Handle negative
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 89a58282b3f..8bfa28e5f6c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1191,6 +1191,10 @@ Wuninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
+Wuniversal-initializer
+C ObjC C++ ObjC++ Var(warn_zero_init) Init(0) Warning
+Don't suppress warnings about { 0 }.
+
 Wmaybe-uninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 50fb1e87ad6..3ade67830e8 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,8 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
+	if warn_zero_init.
+
 2020-06-05  Mark Wielaard  <mark@klomp.org>
 
 	* c-decl.c (implicit_decl_warning): When warned and olddecl is
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 2d04f70f0cf..e1071472301 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8724,7 +8724,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when some structs are initialized with direct aggregation.  */
   if (!implicit && found_missing_braces && warn_missing_braces
-      && !constructor_zeroinit)
+      && (!constructor_zeroinit || warn_zero_init))
     {
       gcc_assert (initializer_stack->missing_brace_richloc);
       warning_at (initializer_stack->missing_brace_richloc,
@@ -8748,8 +8748,9 @@ pop_init_level (location_t loc, int implicit,
 	    /* 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)
+	    /* Do not warn about initializing with { 0 } or with { }
+	       unless warn_zero_init.  */
+	    && (!constructor_zeroinit || warn_zero_init))
 	  {
 	    if (warning_at (input_location, OPT_Wmissing_field_initializers,
 			    "missing initializer for field %qD of %qT",
@@ -8766,7 +8767,7 @@ pop_init_level (location_t loc, int implicit,
     {
       struct location_list *loc = initializer_stack->positional_init_locs;
 
-      if (!constructor_zeroinit)
+      if (!constructor_zeroinit || warn_zero_init)
 	warning_init (loc->loc,
 		      OPT_Wdesignated_init,
 		      "positional initialization of field in %<struct%> "
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 06a04e3d7dd..f0893c250e7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -369,7 +369,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-switch-outside-range  -Wno-switch-unreachable  -Wsync-nand @gol
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
 -Wtype-limits  -Wundef @gol
--Wuninitialized  -Wunknown-pragmas @gol
+-Wuninitialized  -Wno-universal-initializer  -Wunknown-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused @gol
 -Wunused-but-set-parameter  -Wunused-but-set-variable @gol
 -Wunused-const-variable  -Wunused-const-variable=@var{n} @gol
@@ -6453,6 +6453,26 @@ to compute a value that itself is never used, because such
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
+@item -Wuniversal-initializer
+@opindex Wuniversal-initializer
+@opindex Wno-universal-initializer
+Do not suppress warnings about the universal zero initializer,
+@code{@{ 0 @}}, where a warning would otherwise be produced.  For
+example, even with @option{-Wmissing-braces}, the following would not
+warn, because an exception is made for the universal initializer:
+
+@smallexample
+int a[1][1] = @{ 0 @};
+@end smallexample
+
+However, if @code{-Wuniversal-initializer} is used, that code would
+produce a warning (caused by @option{-Wmissing-braces}).
+
+This warning is not enabled by default, nor is it enabled by any other
+options.  If you don't want to suppress warnings about the universal
+zero initializer, you must specify @option{-Wuniversal-initializer}
+explicitly.
+
 @item -Wno-invalid-memory-model
 @opindex Winvalid-memory-model
 @opindex Wno-invalid-memory-model
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 825f60b9d5b..65bd6fd6028 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-07  Asher Gordon  <AsDaGo@posteo.net>
+
+	* gcc.dg/Wuniversal-initializer.c: New test.
+
 2020-06-06  Jan Hubicka  <hubicka@ucw.cz>
 
 	* g++.dg/torture/pr95548.C: New test.
diff --git a/gcc/testsuite/gcc.dg/Wuniversal-initializer.c b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
new file mode 100644
index 00000000000..803dfd10e1c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wmissing-field-initializers -Wuniversal-initializer" } */
+
+struct Pok {
+  int x;
+  int y;
+};
+
+struct Nest {
+  struct Pok p;
+};
+
+struct Des {
+  int a;
+} __attribute__((designated_init));
+
+struct Pok p = { 0 }; /* { dg-warning "missing initializer for field" } */
+struct Nest n = { 0 }; /* { dg-warning "missing braces around initializer" } */
+struct Des d = { 0 }; /* { dg-warning "(positional|near initialization)" } */
+int a[1][1] = { 0 };  /* { dg-warning "missing braces around initializer" } */
-- 
2.26.2


[-- Attachment #1.5: Type: text/plain, Size: 350 bytes --]

Thanks,
Asher

-- 
Violence is the last refuge of the incompetent.
		-- Salvor Hardin
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-04  3:57   ` Asher Gordon
@ 2020-06-08 17:27     ` Martin Sebor
  2020-06-09  1:07       ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Martin Sebor @ 2020-06-08 17:27 UTC (permalink / raw)
  To: Asher Gordon, gcc-patches

On 6/3/20 9:57 PM, Asher Gordon via Gcc-patches wrote:
> Hello,
> 
> I accidentally wrote 'free(loc)' instead of 'free (loc)'. Please see the
> fixed patch attached below (contrib/check_GNU_style.sh says it's OK
> now):
> 
> 
> 0001-Treat-0-specially-for-structs-with-the-designated_in.patch
> 
>  From 0445fba96ee9030feb00ebec893f8dfed153b12d Mon Sep 17 00:00:00 2001
> From: Asher Gordon<AsDaGo@posteo.net>
> Date: Wed, 3 Jun 2020 17:20:08 -0400
> Subject: [PATCH] Treat { 0 } specially for structs with the designated_init
>   attribute.
> 
> Closes #95379.

I think the idea behind this change is fine.  I just have a few
minor comments on the implementation.  A C front end maintainer
will need to formally review the patch.

As an aside, the bug number should be referenced in the change
message so that when the patch is committed it's automatically
reflected in the bug entry.  The contrib/mklog.py script should
do that automatically based on new test files added to verify
the bug fix/change, provided one such test is added and starts
with a PR c/95379 comment.  Also mentioning it in the Subject
of the email is helpful.

> 
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Document { 0 } as a special case for the
> 	designated_init attribute.
> 
> gcc/c/ChangeLog:
> 
> 	* c-typeck.c (struct location_list): New type.
> 	(struct initializer_stack): Add positional_init_locs for
> 	-Wdesignated-init warnings.
> 	(start_init): Initialize initializer_stack->positional_init_locs
> 	to NULL.
> 	(finish_init): Free initializer_stack->positional_init_locs.
> 	(pop_init_level): Move -Wdesignated-init warning here from
> 	process_init_element so that we can treat { 0 } specially.
> 	(process_init_element): Instead of warning on -Wdesignated-init
> 	here, remember a list of locations where we should warn and do the
> 	actual warning in pop_init_level.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.

I don't think there's any "rule" against it but it's customary
to add test files for new features, rather than modify existing
ones (see also my comment about the post-commit hook above).

> ---
>   gcc/ChangeLog                           |  5 +++
>   gcc/c/ChangeLog                         | 14 +++++++

ChangeLog changes need not be included in a patch (and shouldn't
be).  They are now automatically extracted from the commit message
and added to the ChangeLog files by a post-commit hook.


>   gcc/c/c-typeck.c                        | 55 +++++++++++++++++++++++--
>   gcc/doc/extend.texi                     |  4 ++
>   gcc/testsuite/ChangeLog                 |  4 ++
>   gcc/testsuite/gcc.dg/Wdesignated-init.c |  3 ++
>   6 files changed, 81 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index edbcaf2bc4d..fd60a248226 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -95,6 +95,11 @@
>   	(lto_tree_code_to_tag): Update.
>   	(lto_tag_to_tree_code): Update.
>   
> +2020-06-03  Asher Gordon<AsDaGo@posteo.net>
> +
> +	* doc/extend.texi: Document { 0 } as a special case for the
> +	designated_init attribute.
> +
>   2020-06-02  Felix Yang<felix.yang@huawei.com>
>   
>   	PR target/95459
> diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
> index abf31e57688..e77f46930ef 100644
> --- a/gcc/c/ChangeLog
> +++ b/gcc/c/ChangeLog
> @@ -17,6 +17,20 @@
>   
>   	* c-objc-common.h (LANG_HOOKS_OMP_PREDETERMINED_MAPPING): Redefine.
>   
> +2020-06-03  Asher Gordon<AsDaGo@posteo.net>
> +
> +	* c-typeck.c (struct location_list): New type.
> +	(struct initializer_stack): Add positional_init_locs for
> +	-Wdesignated-init warnings.
> +	(start_init): Initialize initializer_stack->positional_init_locs
> +	to NULL.
> +	(finish_init): Free initializer_stack->positional_init_locs.
> +	(pop_init_level): Move -Wdesignated-init warning here from
> +	process_init_element so that we can treat { 0 } specially.
> +	(process_init_element): Instead of warning on -Wdesignated-init
> +	here, remember a list of locations where we should warn and do the
> +	actual warning in pop_init_level.
> +
>   2020-05-28  Nicolas Bértolo<nicolasbertolo@gmail.com>
>   
>   	* Make-lang.in: Remove extra slash.
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 385bf3a1c7b..2d04f70f0cf 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -8179,6 +8179,13 @@ struct constructor_range_stack
>   
>   static struct constructor_range_stack *constructor_range_stack;
>   
> +/* A list of locations.  */
> +
> +struct location_list {
> +  struct location_list *next;
> +  location_t loc;
> +};

Rather than creating own linked list I suggest to consider making
use of one of GCC's data structures.

> +
>   /* This stack records separate initializers that are nested.
>      Nested initializers can't happen in ANSI C, but GNU C allows them
>      in cases like { ... (struct foo) { ... } ... }.  */
> @@ -8197,6 +8204,7 @@ struct initializer_stack
>     char require_constant_value;
>     char require_constant_elements;
>     rich_location *missing_brace_richloc;
> +  struct location_list *positional_init_locs;
>   };
>   
>   static struct initializer_stack *initializer_stack;
> @@ -8222,6 +8230,7 @@ start_init (tree decl, tree asmspec_tree ATTRIBUTE_UNUSED, int top_level,
>     p->top_level = constructor_top_level;
>     p->next = initializer_stack;
>     p->missing_brace_richloc = richloc;
> +  p->positional_init_locs = NULL;
>     initializer_stack = p;
>   
>     constructor_decl = decl;
> @@ -8287,6 +8296,13 @@ finish_init (void)
>     spelling_size = p->spelling_size;
>     constructor_top_level = p->top_level;
>     initializer_stack = p->next;
> +
> +  while (p->positional_init_locs)
> +    {
> +      struct location_list *list = p->positional_init_locs;
> +      p->positional_init_locs = list->next;
> +      free (list);

If the argument to free was returned from XNEW it should probably
be released by XDELETE (even if the two do the same same thing).
But using an existing container class would obviate having to deal
with these details.


> +    }
>     free (p);
>   }
>   \f
> @@ -8744,6 +8760,22 @@ pop_init_level (location_t loc, int implicit,
>   	  }
>       }
>   
> +  /* Warn when positional initializers are used for a structure with
> +     the designated_init attribute, but make an exception for { 0 }.  */
> +  while (initializer_stack->positional_init_locs)
> +    {
> +      struct location_list *loc = initializer_stack->positional_init_locs;
> +
> +      if (!constructor_zeroinit)
> +	warning_init (loc->loc,
> +		      OPT_Wdesignated_init,
> +		      "positional initialization of field in %<struct%> "
> +		      "declared with %<designated_init%> attribute");

I realize the patch doesn't change the text of the message but...
if the name of the field is known at this point, mentioning it in
the message would be a helpful touch.  If it isn't, then "a field"
would be grammatically more correct.  In addition, pointing to
the definition of the struct in a follow-on note would also be
helpful (and in line with other similar messages).  These
improvements are unrelated to the change you're making so could
be made separately, but since you're already making changes here
it's an opportunity to make them now (otherwise they're unlikely
to happen).

Martin

> +
> +      initializer_stack->positional_init_locs = loc->next;
> +      free (loc);
> +    }
> +
>     /* Pad out the end of the structure.  */
>     if (p->replacement_value.value)
>       /* If this closes a superfluous brace pair,
> @@ -9975,10 +10007,25 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
>         && TREE_CODE (constructor_type) == RECORD_TYPE
>         && lookup_attribute ("designated_init",
>   			   TYPE_ATTRIBUTES (constructor_type)))
> -    warning_init (loc,
> -		  OPT_Wdesignated_init,
> -		  "positional initialization of field "
> -		  "in %<struct%> declared with %<designated_init%> attribute");
> +    {
> +      struct location_list *new_loc;
> +
> +      /* Remember where we got a positional initializer so we can warn
> +	 if it initializes a field of a structure with the
> +	 designated_init attribute.  */
> +      new_loc = XNEW (struct location_list);
> +      new_loc->next = NULL;
> +      new_loc->loc = loc;
> +      if (initializer_stack->positional_init_locs)
> +	{
> +	  struct location_list *last = initializer_stack->positional_init_locs;
> +	  while (last->next)
> +	    last++;
> +	  last->next = new_loc;
> +	}
> +      else
> +	initializer_stack->positional_init_locs = new_loc;
> +    }
>   
>     /* If we've exhausted any levels that didn't have braces,
>        pop them now.  */
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index cced19d2018..a3c5059a8b0 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -8093,6 +8093,10 @@ attribute is to allow the programmer to indicate that a structure's
>   layout may change, and that therefore relying on positional
>   initialization will result in future breakage.
>   
> +Note that the universal zero initializer, @code{@{ 0 @}}, is considered
> +a special case and will not produce a warning when used to initialize a
> +structure with the @code{designated_init} attribute.
> +
>   GCC emits warnings based on this attribute by default; use
>   @option{-Wno-designated-init} to suppress them.
>   
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 89a1ad55923..a155c3c5332 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -67,6 +67,10 @@
>   	PR middle-end/94874
>   	* c-c++-common/gomp/pr94874.c: New.
>   
> +2020-06-03  Asher Gordon<AsDaGo@posteo.net>
> +
> +	* gcc.dg/Wdesignated-init.c: Add tests for ignoring { 0 }.
> +
>   2020-06-02  David Malcolm<dmalcolm@redhat.com>
>   
>   	PR jit/95426
> diff --git a/gcc/testsuite/gcc.dg/Wdesignated-init.c b/gcc/testsuite/gcc.dg/Wdesignated-init.c
> index b9ca572206c..adb7949df55 100644
> --- a/gcc/testsuite/gcc.dg/Wdesignated-init.c
> +++ b/gcc/testsuite/gcc.dg/Wdesignated-init.c
> @@ -24,6 +24,9 @@ struct Des {
>   struct Des d1 = { 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
>   struct Des d2 = { .x = 5, .y = 5 };
>   struct Des d3 = { .x = 5, 5 }; /* { dg-warning "(positional|near initialization)" } */
> +struct Des d4 = { 0 };
> +struct Des d5 = { 5 }; /* { dg-warning "(positional|near initialization)" } */
> +struct Des d6 = { 0, 0 }; /* { dg-warning "(positional|near initialization)" } */
>   
>   struct Des fd1 (void)
>   {
> -- 2.26.2
> 
> 
> Thanks,
> Asher
> 
> -- <cas> well there ya go. say something stupid in irc and have it 
> immortalised forever in someone's .sig file -------- I prefer to send 
> and receive mail encrypted. Please send me your public key, and if you 
> do not have my public key, please let me know. Thanks. GPG fingerprint: 
> 38F3 975C D173 4037 B397 8095 D4C9 C4FC 5460 8E68
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-08 17:27     ` Martin Sebor
@ 2020-06-09  1:07       ` Asher Gordon
  2020-06-09  3:31         ` Martin Sebor
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-09  1:07 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 1776 bytes --]

Hi Martin,

Thanks for your helpful comments.

Martin Sebor <msebor@gmail.com> writes:

> As an aside, the bug number should be referenced in the change
> message so that when the patch is committed it's automatically
> reflected in the bug entry.  The contrib/mklog.py script should
> do that automatically based on new test files added to verify
> the bug fix/change, provided one such test is added and starts
> with a PR c/95379 comment.  Also mentioning it in the Subject
> of the email is helpful.
>
> [...]
>
> I don't think there's any "rule" against it but it's customary
> to add test files for new features, rather than modify existing
> ones (see also my comment about the post-commit hook above).
>
> [...]
>
> ChangeLog changes need not be included in a patch (and shouldn't
> be).  They are now automatically extracted from the commit message
> and added to the ChangeLog files by a post-commit hook.

OK, these will be easy to fix. Should my other patch¹ (which implements
-Wuniversal-initializer) also have PR c/95379?

> Rather than creating own linked list I suggest to consider making
> use of one of GCC's data structures.

How would this be done? I read the gccint manual a bit and, from what I
understand, a 'tree' type can be a linked list of other 'tree's. But how
would I make a linked list of generic types ('location_t's)?

> If the argument to free was returned from XNEW it should probably
> be released by XDELETE (even if the two do the same same thing).

In that case, I believe that all other instances of free (which free
something allocated by either XNEW or XRESIZEVEC) should be changed to
XDELETE. The following patch does that (created against the latest
master, without my patches applied).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Replace free with XDELETE --]
[-- Type: text/x-patch, Size: 1990 bytes --]

From 160f696db8e875ab89d6c383ec7f68a772157089 Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Mon, 8 Jun 2020 20:59:38 -0400
Subject: [PATCH] Replace free with XDELETE.

gcc/c/ChangeLog:

	* c-typeck.c (free_all_tagged_tu_seen_up_to): Replace free
	with XDELETE.
	(finish_init): Likewise.
	(pop_init_level): Likewise.
---
 gcc/c/c-typeck.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 385bf3a1c7b..d97dcf50374 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1404,7 +1404,7 @@ free_all_tagged_tu_seen_up_to (const struct tagged_tu_seen_cache *tu_til)
       const struct tagged_tu_seen_cache *const tu1
 	= (const struct tagged_tu_seen_cache *) tu;
       tu = tu1->next;
-      free (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
+      XDELETE (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
     }
   tagged_tu_seen_base = tu_til;
 }
@@ -8268,13 +8268,13 @@ finish_init (void)
     {
       struct constructor_stack *q = constructor_stack;
       constructor_stack = q->next;
-      free (q);
+      XDELETE (q);
     }
 
   gcc_assert (!constructor_range_stack);
 
   /* Pop back to the data of the outer initializer (if any).  */
-  free (spelling_base);
+  XDELETE (spelling_base);
 
   constructor_decl = p->decl;
   require_constant_value = p->require_constant_value;
@@ -8287,7 +8287,7 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
-  free (p);
+  XDELETE (p);
 }
 \f
 /* Call here when we see the initializer is surrounded by braces.
@@ -8818,7 +8818,7 @@ pop_init_level (location_t loc, int implicit,
   RESTORE_SPELLING_DEPTH (constructor_depth);
 
   constructor_stack = p->next;
-  free (p);
+  XDELETE (p);
 
   if (ret.value == NULL_TREE && constructor_stack == 0)
     ret.value = error_mark_node;
-- 
2.26.2


[-- Attachment #1.3: Type: text/plain, Size: 1046 bytes --]

> I realize the patch doesn't change the text of the message but...
> if the name of the field is known at this point, mentioning it in
> the message would be a helpful touch.  If it isn't, then "a field"
> would be grammatically more correct.  In addition, pointing to
> the definition of the struct in a follow-on note would also be
> helpful (and in line with other similar messages).  These
> improvements are unrelated to the change you're making so could
> be made separately, but since you're already making changes here
> it's an opportunity to make them now (otherwise they're unlikely
> to happen).

I will look into this.

Thanks,
Asher

Footnotes: 
¹  https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547460.html

-- 
One picture is worth 128K words.
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-09  1:07       ` Asher Gordon
@ 2020-06-09  3:31         ` Martin Sebor
  2020-06-09 21:21           ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Martin Sebor @ 2020-06-09  3:31 UTC (permalink / raw)
  To: Asher Gordon; +Cc: gcc-patches

On 6/8/20 7:07 PM, Asher Gordon wrote:
> Hi Martin,
> 
> Thanks for your helpful comments.
> 
> Martin Sebor <msebor@gmail.com> writes:
> 
>> As an aside, the bug number should be referenced in the change
>> message so that when the patch is committed it's automatically
>> reflected in the bug entry.  The contrib/mklog.py script should
>> do that automatically based on new test files added to verify
>> the bug fix/change, provided one such test is added and starts
>> with a PR c/95379 comment.  Also mentioning it in the Subject
>> of the email is helpful.
>>
>> [...]
>>
>> I don't think there's any "rule" against it but it's customary
>> to add test files for new features, rather than modify existing
>> ones (see also my comment about the post-commit hook above).
>>
>> [...]
>>
>> ChangeLog changes need not be included in a patch (and shouldn't
>> be).  They are now automatically extracted from the commit message
>> and added to the ChangeLog files by a post-commit hook.
> 
> OK, these will be easy to fix. Should my other patch¹ (which implements
> -Wuniversal-initializer) also have PR c/95379?

I'm sure that would be fine but since it's a separate enhancement
it doesn't need to.  It's up to you.

> 
>> Rather than creating own linked list I suggest to consider making
>> use of one of GCC's data structures.
> 
> How would this be done? I read the gccint manual a bit and, from what I
> understand, a 'tree' type can be a linked list of other 'tree's. But how
> would I make a linked list of generic types ('location_t's)?

I don't think GCC has its own generic linked list container.
There's TREE_CHAIN of tree nodes, but that's probably not
the best choice for this purpose.  I think the vec template
(defined in <vec.h>) would come closest to it (like
constructor_stack::elements).

GCC has its own garbage collector so it's preferable to avoid
manual memory management and all its pitfalls (plus, it's much
more fun to learn about those unique to the garbage collector ;-)

Martin

>> If the argument to free was returned from XNEW it should probably
>> be released by XDELETE (even if the two do the same same thing).
> 
> In that case, I believe that all other instances of free (which free
> something allocated by either XNEW or XRESIZEVEC) should be changed to
> XDELETE. The following patch does that (created against the latest
> master, without my patches applied).
> 
> 
>> I realize the patch doesn't change the text of the message but...
>> if the name of the field is known at this point, mentioning it in
>> the message would be a helpful touch.  If it isn't, then "a field"
>> would be grammatically more correct.  In addition, pointing to
>> the definition of the struct in a follow-on note would also be
>> helpful (and in line with other similar messages).  These
>> improvements are unrelated to the change you're making so could
>> be made separately, but since you're already making changes here
>> it's an opportunity to make them now (otherwise they're unlikely
>> to happen).
> 
> I will look into this.
> 
> Thanks,
> Asher
> 
> Footnotes:
> ¹  https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547460.html
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-09  3:31         ` Martin Sebor
@ 2020-06-09 21:21           ` Asher Gordon
  2020-06-14 17:05             ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-09 21:21 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 2933 bytes --]

Martin Sebor <msebor@gmail.com> writes:

> On 6/8/20 7:07 PM, Asher Gordon wrote:
>>
>> OK, these will be easy to fix. Should my other patch¹ (which implements
>> -Wuniversal-initializer) also have PR c/95379?
>
> I'm sure that would be fine but since it's a separate enhancement
> it doesn't need to.  It's up to you.

I decided not to, because my original report does not deal with
-Wuniversal-initializer. So like you said, it's a separate enhancement.

> I don't think GCC has its own generic linked list container.
> There's TREE_CHAIN of tree nodes, but that's probably not
> the best choice for this purpose.  I think the vec template
> (defined in <vec.h>) would come closest to it (like
> constructor_stack::elements).

I have implemented this. I wasn't sure what the second argument for the
'vec' template should be, so I just used 'va_gc' since that was used for
'elements' in the same structure.

>>> I realize the patch doesn't change the text of the message but...
>>> if the name of the field is known at this point, mentioning it in
>>> the message would be a helpful touch.  If it isn't, then "a field"
>>> would be grammatically more correct.  In addition, pointing to
>>> the definition of the struct in a follow-on note would also be
>>> helpful (and in line with other similar messages).  These
>>> improvements are unrelated to the change you're making so could
>>> be made separately, but since you're already making changes here
>>> it's an opportunity to make them now (otherwise they're unlikely
>>> to happen).
>>
>> I will look into this.

The field name is not known, but it is easy to create a structure to
hold both the location and field name for each positional init. So that
is what I have done, and now the field name is reported in the
warning. Also, instead of just saying 'struct', I also had it refer to
the name of the struct (e.g. 'struct foo') by using '%qT'.

I also added a note after the warning showing where the field was
defined in the structure, like this:

    inform (DECL_SOURCE_LOCATION (info.field),
	    "in definition of %qT", constructor_type);

However, I think it would be preferable to point to the definition of
the structure itself, since that is where the attribute is. Something
like this:

    inform (DECL_SOURCE_LOCATION (constructor_type),
	    "%qT defined here", constructor_type);

Unfortunately, the above does not work since 'constructor_type' is a
type rather than a variable, and thus DECL_SOURCE_LOCATION does not work
on it. Do you know if there is something similar to DECL_SOURCE_LOCATION
that can be used on types like 'constructor_type'?

I also added PR c/95379 to the commit message, and I made the subject
shorter because of the added length of "PR c/95379". The updated patch
can be found below. I'm going to send my updated -Wuniversal-initializer
patch as a reply to that message.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Updated patch --]
[-- Type: text/x-patch, Size: 7908 bytes --]

From 280492c8833c11a3aaf435f0ad63ab8a24a8c584 Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Wed, 3 Jun 2020 17:20:08 -0400
Subject: [PATCH] PR c/95379 Treat { 0 } specially for -Wdesignated-init.

gcc/c/ChangeLog:

	PR c/95379
	* c-typeck.c (warning_init): Allow variable arguments.
	(struct positional_init_info): New type.
	(struct initializer_stack): Add positional_init_info for
	-Wdesignated-init warnings.
	(start_init): Initialize
	initializer_stack->positional_init_info.
	(finish_init): Free initializer_stack->positional_init_info.
	(pop_init_level): Move -Wdesignated-init warning here from
	process_init_element so that we can treat { 0 } specially.
	(process_init_element): Instead of warning on
	-Wdesignated-init here, remember a list of locations and
	fields where we should warn, and do the actual warning in
	pop_init_level.

gcc/ChangeLog:

	PR c/95379
	* doc/extend.texi: Document { 0 } as a special case for the
	designated_init attribute.

gcc/testsuite/ChangeLog:

	PR c/95379
	* gcc.dg/Wdesignated-init-3.c: New test.
---
 gcc/c/c-typeck.c                          | 69 ++++++++++++++++++++---
 gcc/doc/extend.texi                       |  4 ++
 gcc/testsuite/gcc.dg/Wdesignated-init-3.c | 12 ++++
 3 files changed, 77 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wdesignated-init-3.c

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index d97dcf50374..1bae124c6dc 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -104,7 +104,7 @@ static void push_string (const char *);
 static void push_member_name (tree);
 static int spelling_length (void);
 static char *print_spelling (char *);
-static void warning_init (location_t, int, const char *);
+static void warning_init (location_t, int, const char *, ...);
 static tree digest_init (location_t, tree, tree, tree, bool, bool, int);
 static void output_init_element (location_t, tree, tree, bool, tree, tree, bool,
 				 bool, struct obstack *);
@@ -6423,11 +6423,12 @@ pedwarn_init (location_t loc, int opt, const char *gmsgid, ...)
    controls this warning.  GMSGID identifies the message.  The
    component name is taken from the spelling stack.  */
 
-static void
-warning_init (location_t loc, int opt, const char *gmsgid)
+static void ATTRIBUTE_GCC_DIAG (3,4)
+warning_init (location_t loc, int opt, const char *gmsgid, ...)
 {
   char *ofwhat;
   bool warned;
+  va_list ap;
 
   auto_diagnostic_group d;
 
@@ -6437,7 +6438,9 @@ warning_init (location_t loc, int opt, const char *gmsgid)
   location_t exploc = expansion_point_location_if_in_system_header (loc);
 
   /* The gmsgid may be a format string with %< and %>. */
-  warned = warning_at (exploc, opt, gmsgid);
+  va_start (ap, gmsgid);
+  warned = emit_diagnostic_valist (DK_WARNING, exploc, opt, gmsgid, &ap);
+  va_end (ap);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat && warned)
     inform (exploc, "(near initialization for %qs)", ofwhat);
@@ -8179,6 +8182,22 @@ struct constructor_range_stack
 
 static struct constructor_range_stack *constructor_range_stack;
 
+/* This structure stores information for positional initializers
+   (field and location).  */
+struct positional_init_info {
+  location_t loc;
+  tree field;
+
+  /* So that the vector operations can set this to zero.  */
+  void
+  operator= (int value)
+  {
+    gcc_assert (value == 0);
+    loc = 0;
+    field = NULL_TREE;
+  }
+};
+
 /* This stack records separate initializers that are nested.
    Nested initializers can't happen in ANSI C, but GNU C allows them
    in cases like { ... (struct foo) { ... } ... }.  */
@@ -8197,6 +8216,7 @@ struct initializer_stack
   char require_constant_value;
   char require_constant_elements;
   rich_location *missing_brace_richloc;
+  vec<struct positional_init_info, va_gc> *positional_init_info;
 };
 
 static struct initializer_stack *initializer_stack;
@@ -8222,6 +8242,7 @@ start_init (tree decl, tree asmspec_tree ATTRIBUTE_UNUSED, int top_level,
   p->top_level = constructor_top_level;
   p->next = initializer_stack;
   p->missing_brace_richloc = richloc;
+  vec_alloc (p->positional_init_info, 0);
   initializer_stack = p;
 
   constructor_decl = decl;
@@ -8287,6 +8308,8 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
+
+  vec_free (p->positional_init_info);
   XDELETE (p);
 }
 \f
@@ -8744,6 +8767,33 @@ pop_init_level (location_t loc, int implicit,
 	  }
     }
 
+  /* Warn when positional initializers are used for a structure with
+     the designated_init attribute, but make an exception for { 0 }.  */
+  if (!constructor_zeroinit
+      && vec_safe_length (initializer_stack->positional_init_info))
+    {
+      struct positional_init_info info;
+
+      gcc_assert (constructor_type);
+      gcc_assert (TREE_CODE (constructor_type) == RECORD_TYPE);
+
+      for (unsigned int i = 0;
+	   vec_safe_iterate (initializer_stack->positional_init_info, i, &info);
+	   i++)
+	{
+	  warning_init (info.loc,
+			OPT_Wdesignated_init,
+			"positional initialization of field %qD in %qT "
+			"declared with %<designated_init%> attribute",
+			info.field, constructor_type);
+	  inform (DECL_SOURCE_LOCATION (info.field),
+		  "in definition of %qT", constructor_type);
+	}
+    }
+
+  /* We must free this here so we don't warn again later.  */
+  vec_free (initializer_stack->positional_init_info);
+
   /* Pad out the end of the structure.  */
   if (p->replacement_value.value)
     /* If this closes a superfluous brace pair,
@@ -9975,10 +10025,13 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
       && TREE_CODE (constructor_type) == RECORD_TYPE
       && lookup_attribute ("designated_init",
 			   TYPE_ATTRIBUTES (constructor_type)))
-    warning_init (loc,
-		  OPT_Wdesignated_init,
-		  "positional initialization of field "
-		  "in %<struct%> declared with %<designated_init%> attribute");
+    {
+      /* Remember where we got a positional initializer so we can warn
+	 if it initializes a field of a structure with the
+	 designated_init attribute.  */
+      struct positional_init_info info = { loc, constructor_fields };
+      vec_safe_push (initializer_stack->positional_init_info, info);
+    }
 
   /* If we've exhausted any levels that didn't have braces,
      pop them now.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e656e66a80c..70d25d6c779 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8101,6 +8101,10 @@ attribute is to allow the programmer to indicate that a structure's
 layout may change, and that therefore relying on positional
 initialization will result in future breakage.
 
+Note that the universal zero initializer, @code{@{ 0 @}}, is considered
+a special case and will not produce a warning when used to initialize a
+structure with the @code{designated_init} attribute.
+
 GCC emits warnings based on this attribute by default; use
 @option{-Wno-designated-init} to suppress them.
 
diff --git a/gcc/testsuite/gcc.dg/Wdesignated-init-3.c b/gcc/testsuite/gcc.dg/Wdesignated-init-3.c
new file mode 100644
index 00000000000..1da3ed5bef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wdesignated-init-3.c
@@ -0,0 +1,12 @@
+/* PR c/95379 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+struct Des {
+  int x;
+  int y;
+} __attribute__ ((designated_init));
+
+struct Des d1 = { 0 }; /* { dg-bogus "(positional|near initialization)" } */
+struct Des d2 = { 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d3 = { 0, 0 }; /* { dg-warning "(positional|near initialization)" } */
-- 
2.26.2


[-- Attachment #1.3: Type: text/plain, Size: 415 bytes --]

Thanks,
Asher

-- 
Reader, suppose you were an idiot.  And suppose you were a member of
Congress.  But I repeat myself.
                -- Mark Twain
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 }.
  2020-06-07 20:40   ` Asher Gordon
@ 2020-06-09 21:24     ` Asher Gordon
  0 siblings, 0 replies; 29+ messages in thread
From: Asher Gordon @ 2020-06-09 21:24 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 232 bytes --]

As Martin Sebor points out, the ChangeLog entries should not be included
in the patch. So I have removed those. I also made some changes to this
patch that were necessary because of changes to the other patch.

See the patch below:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Updated patch --]
[-- Type: text/x-patch, Size: 5638 bytes --]

From 676b6436835434f89c8511cd68e89947c32f11c6 Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Sun, 7 Jun 2020 13:37:27 -0400
Subject: [PATCH] Add -Wuniversal-initializer to not suppress warnings about {
 0 }.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wuniversal-initializer.

gcc/c-family/ChangeLog:

	* c.opt: Add -Wuniversal-initializer

gcc/c/ChangeLog:

	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
	if warn_zero_init.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wuniversal-initializer.c: New test.
---
 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/c-typeck.c                              |  9 ++++----
 gcc/doc/invoke.texi                           | 22 ++++++++++++++++++-
 gcc/testsuite/gcc.dg/Wuniversal-initializer.c | 20 +++++++++++++++++
 4 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wuniversal-initializer.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 89a58282b3f..8bfa28e5f6c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1191,6 +1191,10 @@ Wuninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
+Wuniversal-initializer
+C ObjC C++ ObjC++ Var(warn_zero_init) Init(0) Warning
+Don't suppress warnings about { 0 }.
+
 Wmaybe-uninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 1bae124c6dc..72efb90c58b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8731,7 +8731,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when some structs are initialized with direct aggregation.  */
   if (!implicit && found_missing_braces && warn_missing_braces
-      && !constructor_zeroinit)
+      && (!constructor_zeroinit || warn_zero_init))
     {
       gcc_assert (initializer_stack->missing_brace_richloc);
       warning_at (initializer_stack->missing_brace_richloc,
@@ -8755,8 +8755,9 @@ pop_init_level (location_t loc, int implicit,
 	    /* 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)
+	    /* Do not warn about initializing with { 0 } or with { }
+	       unless warn_zero_init.  */
+	    && (!constructor_zeroinit || warn_zero_init))
 	  {
 	    if (warning_at (input_location, OPT_Wmissing_field_initializers,
 			    "missing initializer for field %qD of %qT",
@@ -8769,7 +8770,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when positional initializers are used for a structure with
      the designated_init attribute, but make an exception for { 0 }.  */
-  if (!constructor_zeroinit
+  if ((!constructor_zeroinit || warn_zero_init)
       && vec_safe_length (initializer_stack->positional_init_info))
     {
       struct positional_init_info info;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 06a04e3d7dd..f0893c250e7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -369,7 +369,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-switch-outside-range  -Wno-switch-unreachable  -Wsync-nand @gol
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
 -Wtype-limits  -Wundef @gol
--Wuninitialized  -Wunknown-pragmas @gol
+-Wuninitialized  -Wno-universal-initializer  -Wunknown-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused @gol
 -Wunused-but-set-parameter  -Wunused-but-set-variable @gol
 -Wunused-const-variable  -Wunused-const-variable=@var{n} @gol
@@ -6453,6 +6453,26 @@ to compute a value that itself is never used, because such
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
+@item -Wuniversal-initializer
+@opindex Wuniversal-initializer
+@opindex Wno-universal-initializer
+Do not suppress warnings about the universal zero initializer,
+@code{@{ 0 @}}, where a warning would otherwise be produced.  For
+example, even with @option{-Wmissing-braces}, the following would not
+warn, because an exception is made for the universal initializer:
+
+@smallexample
+int a[1][1] = @{ 0 @};
+@end smallexample
+
+However, if @code{-Wuniversal-initializer} is used, that code would
+produce a warning (caused by @option{-Wmissing-braces}).
+
+This warning is not enabled by default, nor is it enabled by any other
+options.  If you don't want to suppress warnings about the universal
+zero initializer, you must specify @option{-Wuniversal-initializer}
+explicitly.
+
 @item -Wno-invalid-memory-model
 @opindex Winvalid-memory-model
 @opindex Wno-invalid-memory-model
diff --git a/gcc/testsuite/gcc.dg/Wuniversal-initializer.c b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
new file mode 100644
index 00000000000..803dfd10e1c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wmissing-field-initializers -Wuniversal-initializer" } */
+
+struct Pok {
+  int x;
+  int y;
+};
+
+struct Nest {
+  struct Pok p;
+};
+
+struct Des {
+  int a;
+} __attribute__((designated_init));
+
+struct Pok p = { 0 }; /* { dg-warning "missing initializer for field" } */
+struct Nest n = { 0 }; /* { dg-warning "missing braces around initializer" } */
+struct Des d = { 0 }; /* { dg-warning "(positional|near initialization)" } */
+int a[1][1] = { 0 };  /* { dg-warning "missing braces around initializer" } */
-- 
2.26.2


[-- Attachment #1.3: Type: text/plain, Size: 485 bytes --]

Thanks,
Asher

-- 
The marvels of today's modern technology include the development of a
soda can, when discarded will last forever ... and a $7,000 car which
when properly cared for will rust out in two or three years.
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-09 21:21           ` Asher Gordon
@ 2020-06-14 17:05             ` Asher Gordon
  2020-06-20 16:37               ` Asher Gordon
  2020-06-20 22:34               ` Martin Sebor
  0 siblings, 2 replies; 29+ messages in thread
From: Asher Gordon @ 2020-06-14 17:05 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

Hello,

Asher Gordon <AsDaGo@posteo.net> writes:

> I also added a note after the warning showing where the field was
> defined in the structure, like this:
>
>     inform (DECL_SOURCE_LOCATION (info.field),
> 	    "in definition of %qT", constructor_type);
>
> However, I think it would be preferable to point to the definition of
> the structure itself, since that is where the attribute is. Something
> like this:
>
>     inform (DECL_SOURCE_LOCATION (constructor_type),
> 	    "%qT defined here", constructor_type);
>
> Unfortunately, the above does not work since 'constructor_type' is a
> type rather than a variable, and thus DECL_SOURCE_LOCATION does not work
> on it. Do you know if there is something similar to DECL_SOURCE_LOCATION
> that can be used on types like 'constructor_type'?

Actually, even that would not be perfect, because the structure may be
defined somewhere and the attribute applied to it elsewhere, even in a
different file. For example:

    struct S {
      int foo, bar;
    };
    struct __attribute__((designated_init)) S;

So it would be better to point to the second declaration of S, because
that is where the attribute is applied. Actually, it would be ideal to
point to the attribute itself, so a note something like the following
could be produced:

    test.c:4:22: note: ‘designated_init’ attribute applied here
        4 | struct __attribute__((designated_init)) S;
          |                       ^~~~~~~~~~~~~~~

Do you know if that's possible?

Thanks,
Asher

-- 
"It's my cookie file and if I come up with something that's lame and I like it,
it goes in."
		-- karl (Karl Lehenbauer)
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-14 17:05             ` Asher Gordon
@ 2020-06-20 16:37               ` Asher Gordon
  2020-06-25 18:09                 ` Joseph Myers
  2020-06-20 22:34               ` Martin Sebor
  1 sibling, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-20 16:37 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

Hi,

Any chance of this patch getting applied soon?

Asher Gordon <AsDaGo@posteo.net> writes:

> Actually, it would be ideal to point to the attribute itself, so a
> note something like the following could be produced:
>
>     test.c:4:22: note: ‘designated_init’ attribute applied here
>         4 | struct __attribute__((designated_init)) S;
>           |                       ^~~~~~~~~~~~~~~
>
> Do you know if that's possible?

If that's not possible, perhaps it would be better to remove the call to
'inform' altogether so as to avoid confusion?

Thanks,
Asher

-- 
He was part of my dream, of course -- but then I was part of his dream too.
		-- Lewis Carroll
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-14 17:05             ` Asher Gordon
  2020-06-20 16:37               ` Asher Gordon
@ 2020-06-20 22:34               ` Martin Sebor
  2020-06-22  4:42                 ` Asher Gordon
  1 sibling, 1 reply; 29+ messages in thread
From: Martin Sebor @ 2020-06-20 22:34 UTC (permalink / raw)
  To: Asher Gordon; +Cc: gcc-patches

On 6/14/20 11:05 AM, Asher Gordon wrote:
> Hello,
> 
> Asher Gordon <AsDaGo@posteo.net> writes:
> 
>> I also added a note after the warning showing where the field was
>> defined in the structure, like this:
>>
>>      inform (DECL_SOURCE_LOCATION (info.field),
>> 	    "in definition of %qT", constructor_type);
>>
>> However, I think it would be preferable to point to the definition of
>> the structure itself, since that is where the attribute is. Something
>> like this:
>>
>>      inform (DECL_SOURCE_LOCATION (constructor_type),
>> 	    "%qT defined here", constructor_type);
>>
>> Unfortunately, the above does not work since 'constructor_type' is a
>> type rather than a variable, and thus DECL_SOURCE_LOCATION does not work
>> on it. Do you know if there is something similar to DECL_SOURCE_LOCATION
>> that can be used on types like 'constructor_type'?

This can be a little confusing and I don't work with the front end
stuff often enough to remember it so I always have to look it up.
There's a TYPE_DECL that corresponds to the definition of the type
that for user-defined types has a source location, otherwise, if T
it's a TYPE_P(T), it doesn't.

I think the TYPE_NAME(T) macro gets the TYPE_DECL for a type T.
I find the best way to figure this out is to think of something
similar to what you're trying to do (like an error or warning in
this case) and look at/step through the code that implements it.

> 
> Actually, even that would not be perfect, because the structure may be
> defined somewhere and the attribute applied to it elsewhere, even in a
> different file. For example:
> 
>      struct S {
>        int foo, bar;
>      };
>      struct __attribute__((designated_init)) S;
> 
> So it would be better to point to the second declaration of S, because
> that is where the attribute is applied. Actually, it would be ideal to
> point to the attribute itself, so a note something like the following
> could be produced:
> 
>      test.c:4:22: note: ‘designated_init’ attribute applied here
>          4 | struct __attribute__((designated_init)) S;
>            |                       ^~~~~~~~~~~~~~~
> 
> Do you know if that's possible?

Unfortunately, attributes don't have locations (yet).  The warning
could have two notes, one pointing to the member and another pointing
to its type.  It's a judgment call what's more important in each case.
It's usually straightforward to find the enclosing type given a note
pointing to a member.

I was under the impression that GCC silently ignored attributes on
type declarations and only respected them on definitions.  But it's
also possible it's inconsistent (attribute handling is still a bit
of a mess).

Martin

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-20 22:34               ` Martin Sebor
@ 2020-06-22  4:42                 ` Asher Gordon
  2020-06-24 20:15                   ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-22  4:42 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3489 bytes --]

Hi Martin,

Thanks for your help.

Martin Sebor <msebor@gmail.com> writes:

> This can be a little confusing and I don't work with the front end
> stuff often enough to remember it so I always have to look it up.
> There's a TYPE_DECL that corresponds to the definition of the type
> that for user-defined types has a source location, otherwise, if T
> it's a TYPE_P(T), it doesn't.
>
> I think the TYPE_NAME(T) macro gets the TYPE_DECL for a type T.
> I find the best way to figure this out is to think of something
> similar to what you're trying to do (like an error or warning in
> this case) and look at/step through the code that implements it.

If I insert an assertion in the code, I find that 'constructor_type' is
indeed a TYPE_P, and TYPE_NAME doesn't work on it. If I try to use
TYPE_NAME, this happens:

    $ install/bin/gcc -c -o test.o test.c
    test.c:14:17: warning: positional initialization of field ‘foo’ in ‘struct S’ declared with ‘designated_init’ attribute [-Wdesignated-init]
       14 | struct S s2 = { 0, 0 };  /* should warn */
          |                 ^
    test.c:14:17: note: (near initialization for ‘s2’)
    test.c:14:8: internal compiler error: tree check: expected tree that contains ‘decl minimal’ structure, have ‘identifier_node’ in pop_init_level, at c/c-typeck.c:8790
       14 | struct S s2 = { 0, 0 };  /* should warn */
          |        ^
    [backtrace]
    $ cat -n gcc/c/c-typeck.c | grep -E -A 3 '^\s*8790\s'
      8790		  inform (DECL_SOURCE_LOCATION
      8791			  (TYPE_NAME
      8792			   (constructor_type)),
      8793			  "in definition of %qT", constructor_type);

So it appears that TYPE_NAME is returning an IDENTIFIER_NODE. Strangely,
the gccint documentation says, referring to 'TYPE_NAME':

    (Note this macro does _not_ return an 'IDENTIFIER_NODE', as you
    might expect, given its name!)

> Unfortunately, attributes don't have locations (yet).

Hmm, well maybe I could implement that. I'm not very familiar with the
GCC source (this is my first patch), but I'll see if I can figure it
out. It would probably be useful for other warnings/errors too.

> The warning could have two notes, one pointing to the member and
> another pointing to its type.  It's a judgment call what's more
> important in each case.  It's usually straightforward to find the
> enclosing type given a note pointing to a member.

If it's not possible to point to the attribute itself, then this is
probably the next best thing. Still, it would be nice to be able to
point directly to the attribute...

> I was under the impression that GCC silently ignored attributes on
> type declarations and only respected them on definitions.  But it's
> also possible it's inconsistent (attribute handling is still a bit
> of a mess).

Yes, actually you're right. I tried defining a struct and then applying
an attribute to a later declaration, and it had no effect. Incidentally,
this seems to me to be confusing behavior, and should probably produce a
warning (or even an error), but that's another issue.

Thanks,
Asher

-- 
Violence is the last refuge of the incompetent.
		-- Salvor Hardin
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-22  4:42                 ` Asher Gordon
@ 2020-06-24 20:15                   ` Asher Gordon
  2020-06-24 23:55                     ` Martin Sebor
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-24 20:15 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 550 bytes --]

Hi Martin,

Asher Gordon <AsDaGo@posteo.net> writes:

> Martin Sebor <msebor@gmail.com> writes:
>
>> Unfortunately, attributes don't have locations (yet).
>
> Hmm, well maybe I could implement that. I'm not very familiar with the
> GCC source (this is my first patch), but I'll see if I can figure it
> out. It would probably be useful for other warnings/errors too.

Well, I've been trying to implement source locations for
IDENTIFIER_NODEs. But I ran into some very strange problems. If I add a
'location_t' for 'struct tree_identifier' like this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Add a 'location_t' to the end of 'struct tree_identifier' --]
[-- Type: text/x-diff, Size: 345 bytes --]

diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 8c5a2e3c404..b3c46d3c0d3 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1423,6 +1423,7 @@ struct GTY(()) tree_poly_int_cst {
 struct GTY(()) tree_identifier {
   struct tree_common common;
   struct ht_identifier id;
+  location_t locus;
 };
 
 struct GTY(()) tree_list {

[-- Attachment #1.3: Type: text/plain, Size: 95 bytes --]

everything works fine. However, if I then try to use the 'location_t' I
just added, like this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: Use IDENTIFIER_SOURCE_LOCATION --]
[-- Type: text/x-diff, Size: 1305 bytes --]

diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index b1cef2345f4..d2d86c5f78a 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -465,6 +465,7 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
 
     case CPP_NAME:
       *value = HT_IDENT_TO_GCC_IDENT (HT_NODE (tok->val.node.node));
+      IDENTIFIER_SOURCE_LOCATION (*value) = *loc;
       break;
 
     case CPP_NUMBER:
diff --git a/gcc/tree.h b/gcc/tree.h
index a74872f5f3e..1e5a4fba0ed 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1069,6 +1069,16 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
 #define IDENTIFIER_HASH_VALUE(NODE) \
   (IDENTIFIER_NODE_CHECK (NODE)->identifier.id.hash_value)
 
+/* The source location in which the identifier appears.  */
+#define IDENTIFIER_SOURCE_LOCATION(NODE) \
+  (IDENTIFIER_NODE_CHECK (NODE)->identifier.locus)
+#define IDENTIFIER_SOURCE_FILE(NODE)\
+  LOCATION_FILE (IDENTIFIER_SOURCE_LOCATION (NODE))
+#define IDENTIFIER_SOURCE_LINE(NODE)\
+  LOCATION_LINE (IDENTIFIER_SOURCE_LOCATION (NODE))
+#define IDENTIFIER_SOURCE_COLUMN(NODE)\
+  LOCATION_COLUMN (IDENTIFIER_SOURCE_LOCATION (NODE))
+
 /* Translate a hash table identifier pointer to a tree_identifier
    pointer, and vice versa.  */
 

[-- Attachment #1.5: Type: text/plain, Size: 132 bytes --]

then lots of ICEs occur. Or, if I don't use IDENTIFIER_SOURCE_LOCATION,
but add 'locus' before 'id' instead of after it, like this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.6: Add a 'location_t' to the middle of 'struct tree_identifier' --]
[-- Type: text/x-diff, Size: 345 bytes --]

diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 8c5a2e3c404..b3c46d3c0d3 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1423,6 +1423,7 @@ struct GTY(()) tree_poly_int_cst {
 struct GTY(()) tree_identifier {
   struct tree_common common;
+  location_t locus;
   struct ht_identifier id;
 };
 
 struct GTY(()) tree_list {

[-- Attachment #1.7: Type: text/plain, Size: 256 bytes --]

then, again, lots of ICEs occur.

My best guess for why the ICEs occur, is that perhaps there is a
'struct' with a similar structure to 'struct tree_identifier', and
pointers to these 'struct's are being casted to each other. Something
like this, sort of:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.8: A program illustrating what could possibly be the problem --]
[-- Type: text/x-csrc, Size: 1167 bytes --]

/* This is analogous to the unknown structure, to and from which
   'struct tree_identifier' is being casted. */
struct foo {
  int i;
  float f;
};

/* This is analogous to the modified 'struct tree_identifier' with the
   'location_t' added at the end. */
struct end {
  int i;
  float f;
  const char *s; /* Analogous to 'locus' in 'struct tree_identifier' */
};

/* This is analogous to the modified 'struct tree_identifier' with the
   'location_t' added in the middle. */
struct middle {
  int i;
  const char *s; /* Analogous to 'locus' in 'struct tree_identifier' */
  float f;
};

int
main (void)
{
  struct foo foo, *foo_p = &foo;
  struct end *end_p;
  struct middle *middle_p;

  /* This works, as long as 's' is not used. */
  end_p = (struct end *) foo_p;
  /* But as soon as 's' is used, it invokes UB. This is analogous to
     the IDENTIFIER_SOURCE_LOCATION in c_lex_with_flags. */
  end_p->s = "hello";

  /* This works, as long as neither 'f' (analogous to 'id') nor 's' is
     used. */
  middle_p = (struct middle *) foo_p;
  /* But, doubtless, 'f' ('id') will be used somewhere... */
  middle_p->f = 42.0;

  return 0; /* if we're lucky... */
}

[-- Attachment #1.9: Type: text/plain, Size: 542 bytes --]

Do you think something like that is possible? And if so, where might it
occur? Or maybe the wrong member of 'union tree_node' is being used
somewhere? But that seems unlikely, since I configured with
--enable-checking...

Thanks,
Asher

-- 
One picture is worth 128K words.
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-24 20:15                   ` Asher Gordon
@ 2020-06-24 23:55                     ` Martin Sebor
  2020-06-25  0:46                       ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Martin Sebor @ 2020-06-24 23:55 UTC (permalink / raw)
  To: Asher Gordon; +Cc: gcc-patches

On 6/24/20 2:15 PM, Asher Gordon wrote:
> Hi Martin,
> 
> Asher Gordon <AsDaGo@posteo.net> writes:
> 
>> Martin Sebor <msebor@gmail.com> writes:
>>
>>> Unfortunately, attributes don't have locations (yet).
>>
>> Hmm, well maybe I could implement that. I'm not very familiar with the
>> GCC source (this is my first patch), but I'll see if I can figure it
>> out. It would probably be useful for other warnings/errors too.
> 
> Well, I've been trying to implement source locations for
> IDENTIFIER_NODEs. But I ran into some very strange problems. If I add a
> 'location_t' for 'struct tree_identifier' like this:
> 
> 
> everything works fine. However, if I then try to use the 'location_t' I
> just added, like this:
> 
> 
> then lots of ICEs occur. Or, if I don't use IDENTIFIER_SOURCE_LOCATION,
> but add 'locus' before 'id' instead of after it, like this:
> 
> 
> then, again, lots of ICEs occur.
> 
> My best guess for why the ICEs occur, is that perhaps there is a
> 'struct' with a similar structure to 'struct tree_identifier', and
> pointers to these 'struct's are being casted to each other. Something
> like this, sort of:
> 
> 
> Do you think something like that is possible? And if so, where might it
> occur? Or maybe the wrong member of 'union tree_node' is being used
> somewhere? But that seems unlikely, since I configured with
> --enable-checking...

I have no experience with changing tree nodes but I wouldn't be
surprised if there were assumptions baked into code that made it
a non-trivial exercise.

There's also lots of sharing of data in GCC so I'm not sure it
makes sense for an identifier to have an associated location.
I imagine two different entities with the same name might share
the same identifier.  It should be easy to verify.  For example
with this test case:

   void f (int i) { }
   void g (int i) { }

and a breakpoint in finish_decl() in c/c-decl.c, the debugger
will stop twice, once for the i in f and then again for the one
in g.  They are two different arguments (with different addresses)
but they both have the same DECL_NAME().

Martin

> 
> Thanks,
> Asher
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-24 23:55                     ` Martin Sebor
@ 2020-06-25  0:46                       ` Asher Gordon
  2020-06-25 17:52                         ` Joseph Myers
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-25  0:46 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]

Martin Sebor <msebor@gmail.com> writes:

> I have no experience with changing tree nodes but I wouldn't be
> surprised if there were assumptions baked into code that made it
> a non-trivial exercise.
>
> There's also lots of sharing of data in GCC so I'm not sure it
> makes sense for an identifier to have an associated location.
> I imagine two different entities with the same name might share
> the same identifier.  It should be easy to verify.  For example
> with this test case:
>
>   void f (int i) { }
>   void g (int i) { }
>
> and a breakpoint in finish_decl() in c/c-decl.c, the debugger
> will stop twice, once for the i in f and then again for the one
> in g.  They are two different arguments (with different addresses)
> but they both have the same DECL_NAME().

I see. So perhaps this isn't the best way to go about implementing
attribute locations. What do you think would be a better way? Perhaps
using a DECL_MINIMAL for attributes?

Thanks,
Asher

-- 
By necessity, by proclivity, and by delight, we all quote.  In fact, it is as
difficult to appropriate the thoughts of others as it is to invent.
		-- R. Emerson
		-- Quoted from a fortune cookie program
		(whose author claims, "Actually, stealing IS easier.")
		[to which I reply, "You think it's easy for me to
		misconstrue all these misquotations?!?"  Ed.]
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-25  0:46                       ` Asher Gordon
@ 2020-06-25 17:52                         ` Joseph Myers
  0 siblings, 0 replies; 29+ messages in thread
From: Joseph Myers @ 2020-06-25 17:52 UTC (permalink / raw)
  To: Asher Gordon; +Cc: Martin Sebor, gcc-patches

On Wed, 24 Jun 2020, Asher Gordon via Gcc-patches wrote:

> I see. So perhaps this isn't the best way to go about implementing
> attribute locations. What do you think would be a better way? Perhaps
> using a DECL_MINIMAL for attributes?

In general, too many things in GCC have the static type "tree".

Ideally identifiers wouldn't be "tree" - there are very few places where 
something can, at runtime, be either an identifier or another kind of 
tree, which makes them a good candidate for a different static type.  But 
there are tricky memory allocation issues around identifiers that make 
changing their representation complicated.

Attributes also have the static type "tree".  They're TREE_LISTs, and 
TREE_LIST is used for many different things and is not a particularly 
efficient type.  Unlike identifiers, there's nothing essential that would 
make it hard to change attributes to have a different static type (which 
could have room to store a location - the parsers know the location for 
each token, it's just that in many cases it ends up getting thrown away 
rather than stored in the datastructures they create) - it would just be a 
large, global change requiring testing across many different targets 
(which should definitely not be combined with any change trying to add a 
new feature - the aim would be that such a change to the internal 
representation does not change how GCC behaves at all).  But each bit of 
that large change should be fairly straightforward.  (I'd guess you might 
have a vec.h vector, each of whose elements is a new attribute type, 
rather than using a linked list with the same type for attributes and 
lists thereof.)

There was at least one previous attempt at changing attributes to have a 
different static type (see commit 4b0b31a65819f64bfeea244bfdcd1a1b8fc3c3cc 
on refs/dead/heads/static-tree-branch), but that was so long ago, and so 
much has changed in GCC since then (including regarding the representation 
of attributes, as part of supporting C++11 attributes and distinguishing 
them from attributes using GNU syntax), that it wouldn't be a useful 
starting point for such a change now.  However, the changes made for 
supporting C++11 attributes would probably make such a change *easier* 
than it was then, because more uses of attributes now go through APIs 
relating specifically to attributes rather than generic TREE_LIST APIs.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-20 16:37               ` Asher Gordon
@ 2020-06-25 18:09                 ` Joseph Myers
  2020-06-25 19:34                   ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Joseph Myers @ 2020-06-25 18:09 UTC (permalink / raw)
  To: Asher Gordon; +Cc: Martin Sebor, gcc-patches

I think both the patches in this discussion (special { 0 } handling and 
the new warning option) generally look good.  I don't see you in the FSF 
copyright assignment list; could you complete 
https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future 
(unless you're already covered by an employer assignment)?

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-25 18:09                 ` Joseph Myers
@ 2020-06-25 19:34                   ` Asher Gordon
  2020-07-22 20:18                     ` Asher Gordon
  0 siblings, 1 reply; 29+ messages in thread
From: Asher Gordon @ 2020-06-25 19:34 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Martin Sebor, gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 433 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> I think both the patches in this discussion (special { 0 } handling
> and the new warning option) generally look good.

I also wrote another small patch, which you might have missed since it's
buried in the discussion here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547554.html
(perhaps I should have sent it as its own message).

Here is the patch below, for convenience:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Replace free with XDELETE --]
[-- Type: text/x-patch, Size: 1990 bytes --]

From 6b18a033ece794088ad7a86bfc557c787c7fc4ae Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Mon, 8 Jun 2020 20:59:38 -0400
Subject: [PATCH] Replace free with XDELETE.

gcc/c/ChangeLog:

	* c-typeck.c (free_all_tagged_tu_seen_up_to): Replace free
	with XDELETE.
	(finish_init): Likewise.
	(pop_init_level): Likewise.
---
 gcc/c/c-typeck.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 3be3690c6e2..fa506b6515c 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1407,7 +1407,7 @@ free_all_tagged_tu_seen_up_to (const struct tagged_tu_seen_cache *tu_til)
       const struct tagged_tu_seen_cache *const tu1
 	= (const struct tagged_tu_seen_cache *) tu;
       tu = tu1->next;
-      free (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
+      XDELETE (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
     }
   tagged_tu_seen_base = tu_til;
 }
@@ -8279,13 +8279,13 @@ finish_init (void)
     {
       struct constructor_stack *q = constructor_stack;
       constructor_stack = q->next;
-      free (q);
+      XDELETE (q);
     }
 
   gcc_assert (!constructor_range_stack);
 
   /* Pop back to the data of the outer initializer (if any).  */
-  free (spelling_base);
+  XDELETE (spelling_base);
 
   constructor_decl = p->decl;
   require_constant_value = p->require_constant_value;
@@ -8298,7 +8298,7 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
-  free (p);
+  XDELETE (p);
 }
 \f
 /* Call here when we see the initializer is surrounded by braces.
@@ -8829,7 +8829,7 @@ pop_init_level (location_t loc, int implicit,
   RESTORE_SPELLING_DEPTH (constructor_depth);
 
   constructor_stack = p->next;
-  free (p);
+  XDELETE (p);
 
   if (ret.value == NULL_TREE && constructor_stack == 0)
     ret.value = error_mark_node;
-- 
2.27.0


[-- Attachment #1.3: Type: text/plain, Size: 891 bytes --]

> I don't see you in the FSF copyright assignment list; could you
> complete
> https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
> (unless you're already covered by an employer assignment)?

Done.

Thanks,
Asher

-- 
By necessity, by proclivity, and by delight, we all quote.  In fact, it is as
difficult to appropriate the thoughts of others as it is to invent.
		-- R. Emerson
		-- Quoted from a fortune cookie program
		(whose author claims, "Actually, stealing IS easier.")
		[to which I reply, "You think it's easy for me to
		misconstrue all these misquotations?!?"  Ed.]
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-06-25 19:34                   ` Asher Gordon
@ 2020-07-22 20:18                     ` Asher Gordon
  2020-08-03 18:47                       ` Asher Gordon
  2020-10-29 21:05                       ` Joseph Myers
  0 siblings, 2 replies; 29+ messages in thread
From: Asher Gordon @ 2020-07-22 20:18 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Martin Sebor, gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 515 bytes --]

Hello Joseph, Martin,

Asher Gordon <AsDaGo@posteo.net> writes:

> Joseph Myers <joseph@codesourcery.com> writes:
>
>> I don't see you in the FSF copyright assignment list; could you
>> complete
>> https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
>> (unless you're already covered by an employer assignment)?
>
> Done.

My copyright assignment finally got finished, so you should be able to
apply my patches now.

For your convenience, I have attached the three patches below:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Replace free with XDELETE --]
[-- Type: text/x-patch, Size: 1994 bytes --]

From e94073b90d5906dc4eb14ebfec4ea24ae1241184 Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Mon, 8 Jun 2020 20:59:38 -0400
Subject: [PATCH 1/3] Replace free with XDELETE.

gcc/c/ChangeLog:

	* c-typeck.c (free_all_tagged_tu_seen_up_to): Replace free
	with XDELETE.
	(finish_init): Likewise.
	(pop_init_level): Likewise.
---
 gcc/c/c-typeck.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index fb5c288b549..44f2722adb8 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1407,7 +1407,7 @@ free_all_tagged_tu_seen_up_to (const struct tagged_tu_seen_cache *tu_til)
       const struct tagged_tu_seen_cache *const tu1
 	= (const struct tagged_tu_seen_cache *) tu;
       tu = tu1->next;
-      free (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
+      XDELETE (CONST_CAST (struct tagged_tu_seen_cache *, tu1));
     }
   tagged_tu_seen_base = tu_til;
 }
@@ -8314,13 +8314,13 @@ finish_init (void)
     {
       struct constructor_stack *q = constructor_stack;
       constructor_stack = q->next;
-      free (q);
+      XDELETE (q);
     }
 
   gcc_assert (!constructor_range_stack);
 
   /* Pop back to the data of the outer initializer (if any).  */
-  free (spelling_base);
+  XDELETE (spelling_base);
 
   constructor_decl = p->decl;
   require_constant_value = p->require_constant_value;
@@ -8333,7 +8333,7 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
-  free (p);
+  XDELETE (p);
 }
 \f
 /* Call here when we see the initializer is surrounded by braces.
@@ -8864,7 +8864,7 @@ pop_init_level (location_t loc, int implicit,
   RESTORE_SPELLING_DEPTH (constructor_depth);
 
   constructor_stack = p->next;
-  free (p);
+  XDELETE (p);
 
   if (ret.value == NULL_TREE && constructor_stack == 0)
     ret.value = error_mark_node;
-- 
2.27.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: PR c/95379 Treat { 0 } specially for -Wdesignated-init --]
[-- Type: text/x-patch, Size: 7913 bytes --]

From b69c8aec5639655bc87ded65d75041a7e1d7c14f Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Wed, 3 Jun 2020 17:20:08 -0400
Subject: [PATCH 2/3] PR c/95379 Treat { 0 } specially for -Wdesignated-init.

gcc/c/ChangeLog:

	PR c/95379
	* c-typeck.c (warning_init): Allow variable arguments.
	(struct positional_init_info): New type.
	(struct initializer_stack): Add positional_init_info for
	-Wdesignated-init warnings.
	(start_init): Initialize
	initializer_stack->positional_init_info.
	(finish_init): Free initializer_stack->positional_init_info.
	(pop_init_level): Move -Wdesignated-init warning here from
	process_init_element so that we can treat { 0 } specially.
	(process_init_element): Instead of warning on
	-Wdesignated-init here, remember a list of locations and
	fields where we should warn, and do the actual warning in
	pop_init_level.

gcc/ChangeLog:

	PR c/95379
	* doc/extend.texi: Document { 0 } as a special case for the
	designated_init attribute.

gcc/testsuite/ChangeLog:

	PR c/95379
	* gcc.dg/Wdesignated-init-3.c: New test.
---
 gcc/c/c-typeck.c                          | 69 ++++++++++++++++++++---
 gcc/doc/extend.texi                       |  4 ++
 gcc/testsuite/gcc.dg/Wdesignated-init-3.c | 12 ++++
 3 files changed, 77 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wdesignated-init-3.c

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 44f2722adb8..1afec3fdc36 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -107,7 +107,7 @@ static void push_string (const char *);
 static void push_member_name (tree);
 static int spelling_length (void);
 static char *print_spelling (char *);
-static void warning_init (location_t, int, const char *);
+static void warning_init (location_t, int, const char *, ...);
 static tree digest_init (location_t, tree, tree, tree, bool, bool, int);
 static void output_init_element (location_t, tree, tree, bool, tree, tree, bool,
 				 bool, struct obstack *);
@@ -6431,11 +6431,12 @@ pedwarn_init (location_t loc, int opt, const char *gmsgid, ...)
    controls this warning.  GMSGID identifies the message.  The
    component name is taken from the spelling stack.  */
 
-static void
-warning_init (location_t loc, int opt, const char *gmsgid)
+static void ATTRIBUTE_GCC_DIAG (3,4)
+warning_init (location_t loc, int opt, const char *gmsgid, ...)
 {
   char *ofwhat;
   bool warned;
+  va_list ap;
 
   auto_diagnostic_group d;
 
@@ -6445,7 +6446,9 @@ warning_init (location_t loc, int opt, const char *gmsgid)
   location_t exploc = expansion_point_location_if_in_system_header (loc);
 
   /* The gmsgid may be a format string with %< and %>. */
-  warned = warning_at (exploc, opt, gmsgid);
+  va_start (ap, gmsgid);
+  warned = emit_diagnostic_valist (DK_WARNING, exploc, opt, gmsgid, &ap);
+  va_end (ap);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat && warned)
     inform (exploc, "(near initialization for %qs)", ofwhat);
@@ -8225,6 +8228,22 @@ struct constructor_range_stack
 
 static struct constructor_range_stack *constructor_range_stack;
 
+/* This structure stores information for positional initializers
+   (field and location).  */
+struct positional_init_info {
+  location_t loc;
+  tree field;
+
+  /* So that the vector operations can set this to zero.  */
+  void
+  operator= (int value)
+  {
+    gcc_assert (value == 0);
+    loc = 0;
+    field = NULL_TREE;
+  }
+};
+
 /* This stack records separate initializers that are nested.
    Nested initializers can't happen in ANSI C, but GNU C allows them
    in cases like { ... (struct foo) { ... } ... }.  */
@@ -8243,6 +8262,7 @@ struct initializer_stack
   char require_constant_value;
   char require_constant_elements;
   rich_location *missing_brace_richloc;
+  vec<struct positional_init_info, va_gc> *positional_init_info;
 };
 
 static struct initializer_stack *initializer_stack;
@@ -8268,6 +8288,7 @@ start_init (tree decl, tree asmspec_tree ATTRIBUTE_UNUSED, int top_level,
   p->top_level = constructor_top_level;
   p->next = initializer_stack;
   p->missing_brace_richloc = richloc;
+  vec_alloc (p->positional_init_info, 0);
   initializer_stack = p;
 
   constructor_decl = decl;
@@ -8333,6 +8354,8 @@ finish_init (void)
   spelling_size = p->spelling_size;
   constructor_top_level = p->top_level;
   initializer_stack = p->next;
+
+  vec_free (p->positional_init_info);
   XDELETE (p);
 }
 \f
@@ -8790,6 +8813,33 @@ pop_init_level (location_t loc, int implicit,
 	  }
     }
 
+  /* Warn when positional initializers are used for a structure with
+     the designated_init attribute, but make an exception for { 0 }.  */
+  if (!constructor_zeroinit
+      && vec_safe_length (initializer_stack->positional_init_info))
+    {
+      struct positional_init_info info;
+
+      gcc_assert (constructor_type);
+      gcc_assert (TREE_CODE (constructor_type) == RECORD_TYPE);
+
+      for (unsigned int i = 0;
+	   vec_safe_iterate (initializer_stack->positional_init_info, i, &info);
+	   i++)
+	{
+	  warning_init (info.loc,
+			OPT_Wdesignated_init,
+			"positional initialization of field %qD in %qT "
+			"declared with %<designated_init%> attribute",
+			info.field, constructor_type);
+	  inform (DECL_SOURCE_LOCATION (info.field),
+		  "in definition of %qT", constructor_type);
+	}
+    }
+
+  /* We must free this here so we don't warn again later.  */
+  vec_free (initializer_stack->positional_init_info);
+
   /* Pad out the end of the structure.  */
   if (p->replacement_value.value)
     /* If this closes a superfluous brace pair,
@@ -10021,10 +10071,13 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
       && TREE_CODE (constructor_type) == RECORD_TYPE
       && lookup_attribute ("designated_init",
 			   TYPE_ATTRIBUTES (constructor_type)))
-    warning_init (loc,
-		  OPT_Wdesignated_init,
-		  "positional initialization of field "
-		  "in %<struct%> declared with %<designated_init%> attribute");
+    {
+      /* Remember where we got a positional initializer so we can warn
+	 if it initializes a field of a structure with the
+	 designated_init attribute.  */
+      struct positional_init_info info = { loc, constructor_fields };
+      vec_safe_push (initializer_stack->positional_init_info, info);
+    }
 
   /* If we've exhausted any levels that didn't have braces,
      pop them now.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 0fb7e27d9ce..332803c64e3 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8101,6 +8101,10 @@ attribute is to allow the programmer to indicate that a structure's
 layout may change, and that therefore relying on positional
 initialization will result in future breakage.
 
+Note that the universal zero initializer, @code{@{ 0 @}}, is considered
+a special case and will not produce a warning when used to initialize a
+structure with the @code{designated_init} attribute.
+
 GCC emits warnings based on this attribute by default; use
 @option{-Wno-designated-init} to suppress them.
 
diff --git a/gcc/testsuite/gcc.dg/Wdesignated-init-3.c b/gcc/testsuite/gcc.dg/Wdesignated-init-3.c
new file mode 100644
index 00000000000..1da3ed5bef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wdesignated-init-3.c
@@ -0,0 +1,12 @@
+/* PR c/95379 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+struct Des {
+  int x;
+  int y;
+} __attribute__ ((designated_init));
+
+struct Des d1 = { 0 }; /* { dg-bogus "(positional|near initialization)" } */
+struct Des d2 = { 5 }; /* { dg-warning "(positional|near initialization)" } */
+struct Des d3 = { 0, 0 }; /* { dg-warning "(positional|near initialization)" } */
-- 
2.27.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: Add -Wuniversal-initializer to not suppress warning --]
[-- Type: text/x-patch, Size: 5642 bytes --]

From acb880883ad4b3256cc5db725c27ebea022e34e9 Mon Sep 17 00:00:00 2001
From: Asher Gordon <AsDaGo@posteo.net>
Date: Sun, 7 Jun 2020 13:37:27 -0400
Subject: [PATCH 3/3] Add -Wuniversal-initializer to not suppress warnings
 about { 0 }.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Wuniversal-initializer.

gcc/c-family/ChangeLog:

	* c.opt: Add -Wuniversal-initializer

gcc/c/ChangeLog:

	* c-typeck.c (pop_init_level): Don't suppress warnings about { 0 }
	if warn_zero_init.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wuniversal-initializer.c: New test.
---
 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/c-typeck.c                              |  9 ++++----
 gcc/doc/invoke.texi                           | 22 ++++++++++++++++++-
 gcc/testsuite/gcc.dg/Wuniversal-initializer.c | 20 +++++++++++++++++
 4 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wuniversal-initializer.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 2b1aca16eb4..4aeed985b8a 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1191,6 +1191,10 @@ Wuninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
 
+Wuniversal-initializer
+C ObjC C++ ObjC++ Var(warn_zero_init) Init(0) Warning
+Don't suppress warnings about { 0 }.
+
 Wmaybe-uninitialized
 C ObjC C++ ObjC++ LTO LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 1afec3fdc36..a9cfe68fd1e 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8777,7 +8777,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when some structs are initialized with direct aggregation.  */
   if (!implicit && found_missing_braces && warn_missing_braces
-      && !constructor_zeroinit)
+      && (!constructor_zeroinit || warn_zero_init))
     {
       gcc_assert (initializer_stack->missing_brace_richloc);
       warning_at (initializer_stack->missing_brace_richloc,
@@ -8801,8 +8801,9 @@ pop_init_level (location_t loc, int implicit,
 	    /* 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)
+	    /* Do not warn about initializing with { 0 } or with { }
+	       unless warn_zero_init.  */
+	    && (!constructor_zeroinit || warn_zero_init))
 	  {
 	    if (warning_at (input_location, OPT_Wmissing_field_initializers,
 			    "missing initializer for field %qD of %qT",
@@ -8815,7 +8816,7 @@ pop_init_level (location_t loc, int implicit,
 
   /* Warn when positional initializers are used for a structure with
      the designated_init attribute, but make an exception for { 0 }.  */
-  if (!constructor_zeroinit
+  if ((!constructor_zeroinit || warn_zero_init)
       && vec_safe_length (initializer_stack->positional_init_info))
     {
       struct positional_init_info info;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ba18e05fb1a..38da2ff0bd5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -371,7 +371,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-switch-outside-range  -Wno-switch-unreachable  -Wsync-nand @gol
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
 -Wtype-limits  -Wundef @gol
--Wuninitialized  -Wunknown-pragmas @gol
+-Wuninitialized  -Wno-universal-initializer  -Wunknown-pragmas @gol
 -Wunsuffixed-float-constants  -Wunused @gol
 -Wunused-but-set-parameter  -Wunused-but-set-variable @gol
 -Wunused-const-variable  -Wunused-const-variable=@var{n} @gol
@@ -6512,6 +6512,26 @@ to compute a value that itself is never used, because such
 computations may be deleted by data flow analysis before the warnings
 are printed.
 
+@item -Wuniversal-initializer
+@opindex Wuniversal-initializer
+@opindex Wno-universal-initializer
+Do not suppress warnings about the universal zero initializer,
+@code{@{ 0 @}}, where a warning would otherwise be produced.  For
+example, even with @option{-Wmissing-braces}, the following would not
+warn, because an exception is made for the universal initializer:
+
+@smallexample
+int a[1][1] = @{ 0 @};
+@end smallexample
+
+However, if @code{-Wuniversal-initializer} is used, that code would
+produce a warning (caused by @option{-Wmissing-braces}).
+
+This warning is not enabled by default, nor is it enabled by any other
+options.  If you don't want to suppress warnings about the universal
+zero initializer, you must specify @option{-Wuniversal-initializer}
+explicitly.
+
 @item -Wno-invalid-memory-model
 @opindex Winvalid-memory-model
 @opindex Wno-invalid-memory-model
diff --git a/gcc/testsuite/gcc.dg/Wuniversal-initializer.c b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
new file mode 100644
index 00000000000..803dfd10e1c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wuniversal-initializer.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wmissing-field-initializers -Wuniversal-initializer" } */
+
+struct Pok {
+  int x;
+  int y;
+};
+
+struct Nest {
+  struct Pok p;
+};
+
+struct Des {
+  int a;
+} __attribute__((designated_init));
+
+struct Pok p = { 0 }; /* { dg-warning "missing initializer for field" } */
+struct Nest n = { 0 }; /* { dg-warning "missing braces around initializer" } */
+struct Des d = { 0 }; /* { dg-warning "(positional|near initialization)" } */
+int a[1][1] = { 0 };  /* { dg-warning "missing braces around initializer" } */
-- 
2.27.0


[-- Attachment #1.5: Type: text/plain, Size: 398 bytes --]

Thanks,
Asher

-- 
<cas> well there ya go.  say something stupid in irc and have it
      immortalised forever in someone's .sig file
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-07-22 20:18                     ` Asher Gordon
@ 2020-08-03 18:47                       ` Asher Gordon
  2020-08-03 20:16                         ` Joseph Myers
  2020-08-25 19:37                         ` Jeff Law
  2020-10-29 21:05                       ` Joseph Myers
  1 sibling, 2 replies; 29+ messages in thread
From: Asher Gordon @ 2020-08-03 18:47 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Martin Sebor, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

Hello,

Asher Gordon <AsDaGo@posteo.net> writes:

> My copyright assignment finally got finished, so you should be able to
> apply my patches now.

Is there any reason my patches haven't been applied yet? Is there
anything else I need to do?

Thanks,
Asher

-- 
<knghtbrd> eek, not another one...
<knghtbrd> Seems ever developer and their mother now has a random
           signature using irc quotes ...
<knghtbrd> WHAT HAVE I STARTED HERE??
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-08-03 18:47                       ` Asher Gordon
@ 2020-08-03 20:16                         ` Joseph Myers
  2020-08-03 21:30                           ` Asher Gordon
  2020-08-25 19:37                         ` Jeff Law
  1 sibling, 1 reply; 29+ messages in thread
From: Joseph Myers @ 2020-08-03 20:16 UTC (permalink / raw)
  To: Asher Gordon; +Cc: gcc-patches

On Mon, 3 Aug 2020, Asher Gordon via Gcc-patches wrote:

> Hello,
> 
> Asher Gordon <AsDaGo@posteo.net> writes:
> 
> > My copyright assignment finally got finished, so you should be able to
> > apply my patches now.
> 
> Is there any reason my patches haven't been applied yet? Is there
> anything else I need to do?

These patches are on my list for review as and when I get time, if no-one 
else gets to them first.  I don't believe there is anything else you need 
to do at present.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-08-03 20:16                         ` Joseph Myers
@ 2020-08-03 21:30                           ` Asher Gordon
  0 siblings, 0 replies; 29+ messages in thread
From: Asher Gordon @ 2020-08-03 21:30 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> These patches are on my list for review as and when I get time, if no-one 
> else gets to them first.  I don't believe there is anything else you need 
> to do at present.

Ah, that's fine. I just wanted to make sure you haven't forgotten about
it. :-)

Thanks,
Asher

-- 
Reader, suppose you were an idiot.  And suppose you were a member of
Congress.  But I repeat myself.
                -- Mark Twain
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-08-03 18:47                       ` Asher Gordon
  2020-08-03 20:16                         ` Joseph Myers
@ 2020-08-25 19:37                         ` Jeff Law
  2020-08-25 20:27                           ` Asher Gordon
  2020-10-15  6:33                           ` Asher Gordon
  1 sibling, 2 replies; 29+ messages in thread
From: Jeff Law @ 2020-08-25 19:37 UTC (permalink / raw)
  To: Asher Gordon, Joseph Myers; +Cc: gcc-patches

On Mon, 2020-08-03 at 14:47 -0400, Asher Gordon via Gcc-patches wrote:
> Hello,
> 
> Asher Gordon <AsDaGo@posteo.net> writes:
> 
> > My copyright assignment finally got finished, so you should be able to
> > apply my patches now.
> 
> Is there any reason my patches haven't been applied yet? Is there
> anything else I need to do?
No, nothing you really need to do.  We're backed up more than usual, but yours is
definitely in the queue.

jeff


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-08-25 19:37                         ` Jeff Law
@ 2020-08-25 20:27                           ` Asher Gordon
  2020-10-15  6:33                           ` Asher Gordon
  1 sibling, 0 replies; 29+ messages in thread
From: Asher Gordon @ 2020-08-25 20:27 UTC (permalink / raw)
  To: Jeff Law; +Cc: Joseph Myers, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

Hi Jeff,

Jeff Law <law@redhat.com> writes:

> On Mon, 2020-08-03 at 14:47 -0400, Asher Gordon via Gcc-patches wrote:
>> Is there any reason my patches haven't been applied yet? Is there
>> anything else I need to do?
> No, nothing you really need to do.  We're backed up more than usual, but yours is
> definitely in the queue.

OK, thanks. Let me know if there's anything I could do to help.

Asher

-- 
I often quote myself; it adds spice to my conversation.
		-- G. B. Shaw
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-08-25 19:37                         ` Jeff Law
  2020-08-25 20:27                           ` Asher Gordon
@ 2020-10-15  6:33                           ` Asher Gordon
  1 sibling, 0 replies; 29+ messages in thread
From: Asher Gordon @ 2020-10-15  6:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: Joseph Myers, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

Hello,

Jeff Law <law@redhat.com> writes:

>> Is there any reason my patches haven't been applied yet? Is there
>> anything else I need to do?
> No, nothing you really need to do.  We're backed up more than usual,
> but yours is definitely in the queue.

It's been a while. Any chance of getting my patch applied soon?

Thanks,
Asher

-- 
A witty saying proves nothing, but saying something pointless gets
people's attention.
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-07-22 20:18                     ` Asher Gordon
  2020-08-03 18:47                       ` Asher Gordon
@ 2020-10-29 21:05                       ` Joseph Myers
  2020-10-30 21:59                         ` Asher Gordon
  1 sibling, 1 reply; 29+ messages in thread
From: Joseph Myers @ 2020-10-29 21:05 UTC (permalink / raw)
  To: Asher Gordon; +Cc: gcc-patches

On Wed, 22 Jul 2020, Asher Gordon via Gcc-patches wrote:

> Hello Joseph, Martin,
> 
> Asher Gordon <AsDaGo@posteo.net> writes:
> 
> > Joseph Myers <joseph@codesourcery.com> writes:
> >
> >> I don't see you in the FSF copyright assignment list; could you
> >> complete
> >> https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
> >> (unless you're already covered by an employer assignment)?
> >
> > Done.
> 
> My copyright assignment finally got finished, so you should be able to
> apply my patches now.
> 
> For your convenience, I have attached the three patches below:

I've tested and committed the first patch.  The second one introduces some 
test failures:

< PASS: gcc.dg/Wdesignated-init-2.c  (test for warnings, line 14)
< PASS: gcc.dg/Wdesignated-init-2.c  (test for warnings, line 15)
---
> FAIL: gcc.dg/Wdesignated-init-2.c  (test for warnings, line 14)
> FAIL: gcc.dg/Wdesignated-init-2.c  (test for warnings, line 15)

< PASS: gcc.dg/init-excess-2.c  (test for warnings, line 30)
---
> FAIL: gcc.dg/init-excess-2.c  (test for warnings, line 30)

Could you investigate those and send versions of the second and third 
patches that don't introduce any test regressions?

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
  2020-10-29 21:05                       ` Joseph Myers
@ 2020-10-30 21:59                         ` Asher Gordon
  0 siblings, 0 replies; 29+ messages in thread
From: Asher Gordon @ 2020-10-30 21:59 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

Joseph Myers <joseph@codesourcery.com> writes:

> I've tested and committed the first patch.

Great, thanks!

> The second one introduces some test failures:
>
> [...]
>
> Could you investigate those and send versions of the second and third
> patches that don't introduce any test regressions?

I've also found a more serious bug: when Wdesignated-init-2.c is
compiled with -Wuniversal-initializer, it causes an internal compiler
error. I'll try to fix this and the test regressions.

-- 
"It's my cookie file and if I come up with something that's lame and I like it,
it goes in."
		-- karl (Karl Lehenbauer)
                               --------
I prefer to send and receive mail encrypted. Please send me your
public key, and if you do not have my public key, please let me
know. Thanks.

GPG fingerprint: 38F3 975C D173 4037 B397  8095 D4C9 C4FC 5460 8E68

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2020-10-30 22:00 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 22:51 [PATCH] Treat { 0 } specially for structs with the designated_init attribute Asher Gordon
2020-06-03 23:03 ` Asher Gordon
2020-06-04  3:57   ` Asher Gordon
2020-06-08 17:27     ` Martin Sebor
2020-06-09  1:07       ` Asher Gordon
2020-06-09  3:31         ` Martin Sebor
2020-06-09 21:21           ` Asher Gordon
2020-06-14 17:05             ` Asher Gordon
2020-06-20 16:37               ` Asher Gordon
2020-06-25 18:09                 ` Joseph Myers
2020-06-25 19:34                   ` Asher Gordon
2020-07-22 20:18                     ` Asher Gordon
2020-08-03 18:47                       ` Asher Gordon
2020-08-03 20:16                         ` Joseph Myers
2020-08-03 21:30                           ` Asher Gordon
2020-08-25 19:37                         ` Jeff Law
2020-08-25 20:27                           ` Asher Gordon
2020-10-15  6:33                           ` Asher Gordon
2020-10-29 21:05                       ` Joseph Myers
2020-10-30 21:59                         ` Asher Gordon
2020-06-20 22:34               ` Martin Sebor
2020-06-22  4:42                 ` Asher Gordon
2020-06-24 20:15                   ` Asher Gordon
2020-06-24 23:55                     ` Martin Sebor
2020-06-25  0:46                       ` Asher Gordon
2020-06-25 17:52                         ` Joseph Myers
2020-06-07 17:50 ` [PATCH] Add -Wuniversal-initializer to not suppress warnings about { 0 } Asher Gordon
2020-06-07 20:40   ` Asher Gordon
2020-06-09 21:24     ` Asher Gordon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).