diff mbox

c/71115 - Missing warning: excess elements in struct initializer

Message ID 573D050A.7030100@gmail.com
State New
Headers show

Commit Message

Martin Sebor May 19, 2016, 12:12 a.m. UTC
The bug points out that the following and similar invalid uses
of NULL are not diagnosed.

   #include <stddef.h>

   const char* a[1] = { "", NULL };

The attached patch implements the suggestion on the Diagnostics
Guidelines Wiki to call
expansion_point_location_if_in_system_header to determine the
location where the macro is used.  Making these changes and
noticing the already existing calls to the function made me
wonder if this approach (warning on system macros) should be
the default strategy, and not warning special.  Aren't there
many more contexts where we would like to see warnings for
them?

In comment #8 on the bug Manuel also suggests to remove the note:
(near initialization for 'decl').  I tried it but decided not to
include it in this change because of the large number of tests it
will require making changes to (I counted at least 20).  I think
it's a worthwhile change but it seems that it might better be
made on its own.

Martin

Comments

Jeff Law May 19, 2016, 7:43 p.m. UTC | #1
On 05/18/2016 06:12 PM, Martin Sebor wrote:
> The bug points out that the following and similar invalid uses
> of NULL are not diagnosed.
>
>   #include <stddef.h>
>
>   const char* a[1] = { "", NULL };
>
> The attached patch implements the suggestion on the Diagnostics
> Guidelines Wiki to call
> expansion_point_location_if_in_system_header to determine the
> location where the macro is used.  Making these changes and
> noticing the already existing calls to the function made me
> wonder if this approach (warning on system macros) should be
> the default strategy, and not warning special.  Aren't there
> many more contexts where we would like to see warnings for
> them?
Seems like it would be a good idea.  Our ability to track this kind of 
stuff has improved greatly over the last several years and a rethink of 
stuff like this is in order.

>
> In comment #8 on the bug Manuel also suggests to remove the note:
> (near initialization for 'decl').  I tried it but decided not to
> include it in this change because of the large number of tests it
> will require making changes to (I counted at least 20).  I think
> it's a worthwhile change but it seems that it might better be
> made on its own.
Yes, that seems like a separate change.

As for the patch, it's fine for the trunk.  I note that the BZ entry 
claims this is a regression since 4.8, so rather than close, just remove 
the "7" from the regression marker.

Jeff
diff mbox

Patch

PR c/71115 - Missing warning: excess elements in struct initializer

gcc/testsuite/ChangeLog:
2016-05-18  Martin Sebor  <msebor@redhat.com>

	PR c/71115
	* gcc.dg/init-excess-2.c: New test.

gcc/c/ChangeLog:
2016-05-18  Martin Sebor  <msebor@redhat.com>

	PR c/71115
	* c-typeck.c (error_init): Use
	expansion_point_location_if_in_system_header.
	(warning_init): Same.

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 0fa9653..2abb171 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -5871,16 +5871,21 @@  error_init (location_t loc, const char *gmsgid)
    component name is taken from the spelling stack.  */
 
 static void
-pedwarn_init (location_t location, int opt, const char *gmsgid)
+pedwarn_init (location_t loc, int opt, const char *gmsgid)
 {
   char *ofwhat;
   bool warned;
 
+  /* Use the location where a macro was expanded rather than where
+     it was defined to make sure macros defined in system headers
+     but used incorrectly elsewhere are diagnosed.  */
+  source_location exploc = expansion_point_location_if_in_system_header (loc);
+
   /* The gmsgid may be a format string with %< and %>. */
-  warned = pedwarn (location, opt, gmsgid);
+  warned = pedwarn (exploc, opt, gmsgid);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat && warned)
-    inform (location, "(near initialization for %qs)", ofwhat);
+    inform (exploc, "(near initialization for %qs)", ofwhat);
 }
 
 /* Issue a warning for a bad initializer component.
@@ -5895,11 +5900,16 @@  warning_init (location_t loc, int opt, const char *gmsgid)
   char *ofwhat;
   bool warned;
 
+  /* Use the location where a macro was expanded rather than where
+     it was defined to make sure macros defined in system headers
+     but used incorrectly elsewhere are diagnosed.  */
+  source_location exploc = expansion_point_location_if_in_system_header (loc);
+
   /* The gmsgid may be a format string with %< and %>. */
-  warned = warning_at (loc, opt, gmsgid);
+  warned = warning_at (exploc, opt, gmsgid);
   ofwhat = print_spelling ((char *) alloca (spelling_length () + 1));
   if (*ofwhat && warned)
-    inform (loc, "(near initialization for %qs)", ofwhat);
+    inform (exploc, "(near initialization for %qs)", ofwhat);
 }
 
 /* If TYPE is an array type and EXPR is a parenthesized string
diff --git a/gcc/testsuite/gcc.dg/init-excess-2.c b/gcc/testsuite/gcc.dg/init-excess-2.c
new file mode 100644
index 0000000..1bf0a96
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/init-excess-2.c
@@ -0,0 +1,47 @@ 
+/* Test for diagnostics about excess initializers when using a macro
+   defined in a system header:
+   c/71115 - Missing warning: excess elements in struct initializer.  */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+#include <stddef.h>
+
+int* a[1] = {
+  0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};
+
+const char str[1] = {
+  0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};
+
+struct S {
+  int *a;
+} s = {
+  0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};
+
+struct __attribute__ ((designated_init)) S2 {
+  int *a;
+} s2 = {
+  NULL              /* { dg-warning "positional initialization|near init" } */
+};
+
+union U {
+  int *a;
+} u = {
+  0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};
+
+int __attribute__ ((vector_size (16))) ivec = {
+  0, 0, 0, 0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};
+
+int* scal = {
+  0,
+  NULL              /* { dg-warning "excess elements|near init" } */
+};