Summary: | [9 Regression] Rejects valid? C code since r259641 | ||
---|---|---|---|
Product: | gcc | Reporter: | Martin Liška <marxin> |
Component: | c | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | jakub, jsm28, msebor |
Priority: | P3 | Keywords: | rejects-valid |
Version: | 9.0 | ||
Target Milestone: | 9.0 | ||
See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88773 | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: |
Description
Martin Liška
2019-01-09 10:26:08 UTC
Reduced testcase: struct S { int s; }; void foo (void) { void *p = &(struct S) { 0 }; void *q = &({ (struct S) { 0 }; }); } The p initializer is accepted, q is rejected. By my reading this is invalid, C99 6.5.2.5/6 says: "If the compound literal occurs outside the body of a function, the object has static storage duration; otherwise, it has automatic storage duration associated with the enclosing block." and the statement expression is still a compound statement and thus the compound literal is associated with the statement expression's block. So it is the same thing as: void bar (void) { void *r = &({ int a = 0; a; }); } which fails with the same diagnostics. Joseph, do you agree? Yes, I think that (a) a statement expression is not an lvalue and (b) if it were (or if the code were changed to move the unary '&' inside the statement expression), the code would be taking the address of an object whose lifetime had ended by the time that address is used. gpg2 needs to be fixed then. Looking at the source, they wrap the complit in: #define DNS_PRAGMA_PUSH _Pragma("GCC diagnostic push") #define DNS_PRAGMA_QUIET _Pragma("GCC diagnostic ignored \"-Woverride-init\"") #define DNS_PRAGMA_POP _Pragma("GCC diagnostic pop") /* GCC parses the _Pragma operator less elegantly than clang. */ #define dns_quietinit(...) \ __extension__ ({ DNS_PRAGMA_PUSH DNS_PRAGMA_QUIET __VA_ARGS__; DNS_PRAGMA_POP }) while for clang they use #define dns_quietinit(...) \ DNS_PRAGMA_PUSH DNS_PRAGMA_QUIET __VA_ARGS__ DNS_PRAGMA_POP Trying: #define DNS_PRAGMA_PUSH _Pragma("GCC diagnostic push") #define DNS_PRAGMA_QUIET _Pragma("GCC diagnostic ignored \"-Woverride-init\"") #define DNS_PRAGMA_POP _Pragma("GCC diagnostic pop") #define dns_quietinit(...) \ DNS_PRAGMA_PUSH DNS_PRAGMA_QUIET __VA_ARGS__ DNS_PRAGMA_POP struct S { int a, b; }; void foo (void) { void *p = &(struct S) { .a = 0, .b = 1, .a = 0 }; void *q = &dns_quietinit ((struct S) { .a = 0, .b = 1, .a = 0 }); } indeed fails to parse: /tmp/h.c: In function ‘foo’: /tmp/h.c:13:48: warning: initialized field overwritten [-Woverride-init] void *p = &(struct S) { .a = 0, .b = 1, .a = 0 }; ^ /tmp/h.c:13:48: note: (near initialization for ‘(anonymous).a’) /tmp/h.c:14:1: error: expected expression before ‘#pragma’ void *q = &dns_quietinit ((struct S) { .a = 0, .b = 1, .a = 0 }); ^ ~ (the line 13 warnings are expected and line 14 is their attempt to disable the warning. We don't really support pragmas in the middle of expressions, but a compound literal can't be used here. I don't disagree with the conclusion about the validity of this test case but there are examples where GCC does treat a statement expression as an lvalue, such as in the following: void f (void) { int i = 0; ++__extension__({ (int)i; }); // accepted if (i != 1) // folded to false __builtin_abort (); } Accepting this seems like a bug irrespective of whether statement expressions themselves are ever lvalues. GCC of course also accepts ++({ i; }). Joseph's comment #2 suggests this should be invalid as well but I might be reading too much into it. In any case, it would be helpful to make this clear in the manual. On Wed, 9 Jan 2019, msebor at gcc dot gnu.org wrote: > I don't disagree with the conclusion about the validity of this test case but > there are examples where GCC does treat a statement expression as an lvalue, > such as in the following: > > void f (void) > { > int i = 0; > ++__extension__({ (int)i; }); // accepted > if (i != 1) // folded to false > __builtin_abort (); > } > > Accepting this seems like a bug irrespective of whether statement expressions > themselves are ever lvalues. Indeed. Casts as lvalues were removed a very long time ago. > GCC of course also accepts ++({ i; }). Joseph's comment #2 suggests this > should be invalid as well but I might be reading too much into it. In any > case, it would be helpful to make this clear in the manual. I think ++({ i; }) should be invalid. (If i is an _Atomic int, it's already rejected because of the special lvalue-to-rvalue processing applied for atomics.) As for the GCC diagnostic pragma, we need it to be a deferred pragma, have no idea how else could we handle that when it is not visible in the token stream. And, if it is visible in the token stream, accepting it everywhere in the exceptions would be quite hard too, e.g. the C FE normally has only 2 tokens lookahead, or how would it play in C++ FE with the tentative parsing? What we could do is accept #pragma GCC diagnostic (and only that?) inside of the braced initializers (so c_parser_braced_init for C and cp_parser_braced_list? for C++), before each init elt and after those, perhaps rejecting it if it appears before a comma. Of course, for gnupg2 they would need to adjust their macros slightly, because the would need to go inside of the compound literal braced init rather than around it. Thoughts on that? A C proof of concept: --- gcc/c/c-parser.c.jj 2019-01-07 09:47:33.187515618 +0100 +++ gcc/c/c-parser.c 2019-01-09 19:15:05.216756760 +0100 @@ -4624,6 +4624,11 @@ c_parser_braced_init (c_parser *parser, } else really_start_incremental_init (type); + + /* Accept #pragmas at braced init scope. */ + while (c_parser_next_token_is (parser, CPP_PRAGMA)) + c_parser_pragma (parser, pragma_struct, NULL); + if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE)) { pedwarn (brace_loc, OPT_Wpedantic, "ISO C forbids empty initializer braces"); @@ -4634,6 +4639,10 @@ c_parser_braced_init (c_parser *parser, comma. */ while (true) { + /* Accept #pragmas at braced init scope. */ + while (c_parser_next_token_is (parser, CPP_PRAGMA)) + c_parser_pragma (parser, pragma_struct, NULL); + c_parser_initelt (parser, &braced_init_obstack); if (parser->error) break; @@ -4644,10 +4653,20 @@ c_parser_braced_init (c_parser *parser, } else break; + + /* Accept #pragmas at braced init scope. */ + while (c_parser_next_token_is (parser, CPP_PRAGMA)) + c_parser_pragma (parser, pragma_struct, NULL); + if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE)) break; } } + + /* Accept #pragmas at braced init scope. */ + while (c_parser_next_token_is (parser, CPP_PRAGMA)) + c_parser_pragma (parser, pragma_struct, NULL); + c_token *next_tok = c_parser_peek_token (parser); if (next_tok->type != CPP_CLOSE_BRACE) { Of course, this one parses all pragmas, rather than just GCC diagnostic, and for most pragmas handling them in this context is undesirable (either meaningful, or e.g. OpenMP/OpenACC forbids them there). |