Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Joseph Myers 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On Wed, 22 Jul 2020, Asher Gordon via Gcc-patches wrote: > Hello Joseph, Martin, > > Asher Gordon writes: > > > Joseph Myers 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 jos...@codesourcery.com
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hello, Jeff Law 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hi Jeff, Jeff Law 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On Mon, 2020-08-03 at 14:47 -0400, Asher Gordon via Gcc-patches wrote: > Hello, > > Asher Gordon 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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Joseph Myers 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On Mon, 3 Aug 2020, Asher Gordon via Gcc-patches wrote: > Hello, > > Asher Gordon 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 jos...@codesourcery.com
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hello, Asher Gordon 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 -- eek, not another one... Seems ever developer and their mother now has a random signature using irc quotes ... 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hello Joseph, Martin,
Asher Gordon writes:
> Joseph Myers 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:
From e94073b90d5906dc4eb14ebfec4ea24ae1241184 Mon Sep 17 00:00:00 2001
From: Asher Gordon
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);
}
/* 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
From b69c8aec5639655bc87ded65d75041a7e1d7c14f Mon Sep 17 00:00:00 2001
From: Asher Gordon
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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Joseph Myers 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:
From 6b18a033ece794088ad7a86bfc557c787c7fc4ae Mon Sep 17 00:00:00 2001
From: Asher Gordon
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);
}
/* 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
> 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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
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
jos...@codesourcery.com
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
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 jos...@codesourcery.com
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Martin Sebor 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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On 6/24/20 2:15 PM, Asher Gordon wrote:
Hi Martin,
Asher Gordon writes:
Martin Sebor 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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hi Martin,
Asher Gordon writes:
> Martin Sebor 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:
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 {
everything works fine. However, if I then try to use the 'location_t' I
just added, like this:
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. */
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:
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 {
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:
/* 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 =
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... */
}
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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hi Martin,
Thanks for your help.
Martin Sebor 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'
8790inform (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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On 6/14/20 11:05 AM, Asher Gordon wrote:
Hello,
Asher Gordon 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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hi, Any chance of this patch getting applied soon? Asher Gordon 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 signature.asc Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hello,
Asher Gordon 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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Martin Sebor 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 ) 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
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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
On 6/8/20 7:07 PM, Asher Gordon wrote:
Hi Martin,
Thanks for your helpful comments.
Martin Sebor 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 ) 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
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Hi Martin,
Thanks for your helpful comments.
Martin Sebor 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).
From 160f696db8e875ab89d6c383ec7f68a772157089 Mon Sep 17 00:00:00 2001
From: Asher Gordon
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);
}
/* 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
> 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
signature.asc
Description: PGP signature
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
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
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
+
+ * doc/extend.texi: Document { 0 } as a special case for the
+ designated_init attribute.
+
2020-06-02 Felix Yang
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
+
+ * 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
* 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;
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
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):
From 0445fba96ee9030feb00ebec893f8dfed153b12d Mon Sep 17 00:00:00 2001
From: Asher Gordon
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
+
+ * doc/extend.texi: Document { 0 } as a special case for the
+ designated_init attribute.
+
2020-06-02 Felix Yang
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
+
+ * 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
* 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);
}
@@ -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 % "
+ "declared with % attribute");
+
+ initializer_stack->positional_init_locs = loc->next;
+ free (loc);
+}
Re: [PATCH] Treat { 0 } specially for structs with the designated_init attribute.
Asher Gordon 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 signature.asc Description: PGP signature
[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 }.
---
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
+
+ * doc/extend.texi: Document { 0 } as a special case for the
+ designated_init attribute.
+
2020-06-02 Felix Yang
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
+
+ * 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
* 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);
}
@@ -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 % "
+ "declared with % 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
