Bug 89640

Summary: [9 Regression] g++ chokes on lambda with __attribute__
Product: gcc Reporter: Mathias Stearn <redbeard0531>
Component: c++Assignee: Jason Merrill <jason>
Status: RESOLVED FIXED    
Severity: normal CC: jakub, jason, ttimo, webrown.cpp
Priority: P3 Keywords: c++-lambda, rejects-valid
Version: 9.0   
Target Milestone: 9.0   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2019-03-11 00:00:00
Bug Depends on:    
Bug Blocks: 54367    

Description Mathias Stearn 2019-03-09 00:46:06 UTC
Regression in gcc 9 vs 8.

https://godbolt.org/z/MGL0H4

void test() {
    []() __attribute__((noinline,cold)) {
        asm volatile("");
    }();
}

<source>: In function 'void test()':
<source>:2:41: error: expected identifier before '{' token
    2 |     []() __attribute__((noinline,cold)) {
      |                                         ^
<source>:2:41: error: type-specifier invalid in lambda
Compiler returned: 1
Comment 1 Jonathan Wakely 2019-03-11 12:37:18 UTC
This is accepted:

  []() [[gnu::noinline,gnu::cold]] 

but it warns that the attributes are ignored, which still isn't right.
Comment 2 Mathias Stearn 2019-03-11 14:08:34 UTC
Unfortunately the c++ attributes syntax applies to the lambda type rather than the function, so the warning is correct. The old style __attribute__ syntax seems to be the only way to annotate the lambda function, which is why we use it here. We use something like this in a macro around code that builds error messages on our error paths, which is why it needs to be on a lambda. It made a notable shrink to the size of our primary .text section moving that stuff out to the .text.cold section.
Comment 3 Jakub Jelinek 2019-03-11 16:43:14 UTC
This changed with r265787 aka PR60503 fix and from that it seems it was completely intentional.
Comment 4 Mathias Stearn 2019-03-11 18:22:05 UTC
@Jakub, This code doesn't have either mutable or noexcept, so the "wrong place in the grammer" issue doesn't apply. That part of the fix seems correct and useful.

While it seems correct to fix what the c++11-syle [[attribute]] appertains to to match the standard, it would be unfortunate to do the same to __attribute__(()) style attributes which are already outside of the standard. Until the standard provides some way to have an attribute that appertains to the lambda function, this part of the "fix" is breaking useful functionality that has existed since GCC-5 without offering any replacement.
Comment 5 Jason Merrill 2019-03-18 19:35:22 UTC
Author: jason
Date: Mon Mar 18 19:34:47 2019
New Revision: 269775

URL: https://gcc.gnu.org/viewcvs?rev=269775&root=gcc&view=rev
Log:
	PR c++/89640 - GNU attributes on lambda.

My patch for PR 60503 to fix C++11 attribute parsing on lambdas accidentally
removed support for GNU attributes.

	* parser.c (cp_parser_lambda_declarator_opt): Allow GNU attributes.

Added:
    trunk/gcc/testsuite/g++.dg/ext/attr-lambda1.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/parser.c
Comment 6 Jason Merrill 2019-03-18 20:36:29 UTC
Fixed.
Comment 7 frankhb1989 2019-06-23 08:12:32 UTC
GCC 9.1 still does not work when there is a trailing-return-type:

void test() {
    []() __attribute__((noreturn)) -> int{
        return 0; // Warning expected.
    }();
}

This looks like the same regression.
Comment 8 Jakub Jelinek 2019-11-02 06:54:25 UTC
Author: jakub
Date: Sat Nov  2 06:53:53 2019
New Revision: 277741

URL: https://gcc.gnu.org/viewcvs?rev=277741&root=gcc&view=rev
Log:
	PR c++/89640
	* parser.c (cp_parser_decl_specifier_seq): Don't parse attributes
	if CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR.

	* g++.dg/cpp1z/attr-lambda1.C: New test.
	* g++.dg/ext/attr-lambda2.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/cpp1z/attr-lambda1.C
    trunk/gcc/testsuite/g++.dg/ext/attr-lambda2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/parser.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 frankhb1989 2019-11-28 18:53:40 UTC
This seems still problematic.

void test1() {
    []() __attribute__((noreturn)) noexcept [[]] -> int{
        return 0; // Warning expected.
    }();
}

void test2() {
    []() noexcept [[]] __attribute__((noreturn)) -> int{
        return 0; // Warning expected.
    }();
}

Clang++ 9 accepts test1 but not test2. (However, it issues an error instead of a warning.) Both fail in trunk G++.
Are they expected work?
Comment 10 GCC Commits 2020-01-29 22:51:00 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:245e40af4fab5b7cf40fb310591a879355775971

commit r10-6335-g245e40af4fab5b7cf40fb310591a879355775971
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jan 28 17:41:05 2020 -0500

    c++: Fix attributes with lambda and trailing return type.
    
    My fix for 60503 fixed handling of C++11 attributes following the
    lambda-declarator.  My patch for 89640 re-added support for GNU attributes,
    but attributes after the trailing return type were parsed as applying to the
    return type rather than to the function.  This patch adjusts parsing of a
    trailing-return-type to ignore GNU attributes at the end of the declaration
    so that they will be applied to the declaration as a whole.
    
    I also considered parsing the attributes between the closing paren and the
    trailing-return-type, and tried a variety of approaches to implementing
    that, but I think it's better to stick with the documented rule that "An
    attribute specifier list may appear immediately before the comma, '=' or
    semicolon terminating the declaration of an identifier...."  Anyone
    disagree?
    
    Meanwhile, C++ committee discussion about the lack of any way to apply
    attributes to a lambda op() seems to have concluded that they should go
    between the introducer and declarator, so I've implemented that as well.
    
    	PR c++/90333
    	PR c++/89640
    	PR c++/60503
    	* parser.c (cp_parser_type_specifier_seq): Don't parse attributes in
    	a trailing return type.
    	(cp_parser_lambda_declarator_opt): Parse C++11 attributes before
    	parens.
Comment 11 GCC Commits 2020-03-02 21:25:54 UTC
The releases/gcc-9 branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:166c024a1969ca9e77ed450fb65ce5c926a315dc

commit r9-8318-g166c024a1969ca9e77ed450fb65ce5c926a315dc
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Mar 2 14:42:47 2020 -0500

    c++: Fix attributes with lambda and trailing return type.
    
    My fix for 60503 fixed handling of C++11 attributes following the
    lambda-declarator.  My patch for 89640 re-added support for GNU attributes,
    but attributes after the trailing return type were parsed as applying to the
    return type rather than to the function.  This patch adjusts parsing of a
    trailing-return-type to ignore GNU attributes at the end of the declaration
    so that they will be applied to the declaration as a whole.
    
    I also considered parsing the attributes between the closing paren and the
    trailing-return-type, and tried a variety of approaches to implementing
    that, but I think it's better to stick with the documented rule that "An
    attribute specifier list may appear immediately before the comma, '=' or
    semicolon terminating the declaration of an identifier...."  Anyone
    disagree?
    
    Meanwhile, C++ committee discussion about the lack of any way to apply
    attributes to a lambda op() seems to have concluded that they should go
    between the introducer and declarator, so I've implemented that as well.
    
    gcc/cp/ChangeLog
    2020-03-02  Jason Merrill  <jason@redhat.com>
    
    	PR c++/90333
    	PR c++/89640
    	PR c++/60503
    	* parser.c (cp_parser_type_specifier_seq): Don't parse attributes in
    	a trailing return type.
    	(cp_parser_lambda_declarator_opt): Parse C++11 attributes before
    	parens.
Comment 12 Timothee Besset 2020-06-25 21:53:47 UTC
This is still a problem in 9.3 and 10 when using a return type:

void test() {
    []() __attribute__((noinline)) -> int {
      return 0;
    }();
}
Comment 13 Timothee Besset 2020-06-25 23:33:21 UTC
Fix proposed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95883