Martin Sebor <mse...@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.
From 280492c8833c11a3aaf435f0ad63ab8a24a8c584 Mon Sep 17 00:00:00 2001
From: Asher Gordon <asd...@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);
}
@@ -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
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
signature.asc
Description: PGP signature
