Skip to content

[clang] Accept lambdas in C++03 as an extensions #73376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

philnik777
Copy link
Contributor

@philnik777 philnik777 commented Nov 25, 2023

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 25, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2023

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: None (philnik777)

Changes

This is a fairly simple extension, but makes the life for people who
have to support C++03 a lot easier. As a nice bonus, this also improves
diagnostics, since lambdas are now properly recognized when parsing
C++03 code.


Full diff: https://github.com/llvm/llvm-project/pull/73376.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+1)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+1-1)
  • (modified) clang/lib/Parse/ParseExprCXX.cpp (+3-1)
  • (modified) clang/lib/Parse/ParseInit.cpp (+1-1)
  • (modified) clang/test/Parser/cxx0x-lambda-expressions.cpp (+47-69)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index b66ecf0724b1c77..c8616cacda62280 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1030,6 +1030,7 @@ def err_capture_default_first : Error<
 def ext_decl_attrs_on_lambda : ExtWarn<
   "%select{an attribute specifier sequence|%0}1 in this position "
   "is a C++23 extension">, InGroup<CXX23>;
+def ext_lambda : ExtWarn<"lambdas are a C++11 extension">, InGroup<CXX11>;
 def ext_lambda_missing_parens : ExtWarn<
   "lambda without a parameter clause is a C++23 extension">,
   InGroup<CXX23>;
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 897810557976151..c453d77329111f6 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1799,7 +1799,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
     }
     goto ExpectedExpression;
   case tok::l_square:
-    if (getLangOpts().CPlusPlus11) {
+    if (getLangOpts().CPlusPlus) {
       if (getLangOpts().ObjC) {
         // C++11 lambda expressions and Objective-C message sends both start with a
         // square bracket.  There are three possibilities here:
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 79db094e098f8e6..9b459e8a4d3f4b2 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1271,7 +1271,9 @@ static void DiagnoseStaticSpecifierRestrictions(Parser &P,
 ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
                      LambdaIntroducer &Intro) {
   SourceLocation LambdaBeginLoc = Intro.Range.getBegin();
-  Diag(LambdaBeginLoc, diag::warn_cxx98_compat_lambda);
+  Diag(LambdaBeginLoc, getLangOpts().CPlusPlus11
+                           ? diag::warn_cxx98_compat_lambda
+                           : diag::ext_lambda);
 
   PrettyStackTraceLoc CrashInfo(PP.getSourceManager(), LambdaBeginLoc,
                                 "lambda expression parsing");
diff --git a/clang/lib/Parse/ParseInit.cpp b/clang/lib/Parse/ParseInit.cpp
index 637f21176792b6b..423497bfcb6621a 100644
--- a/clang/lib/Parse/ParseInit.cpp
+++ b/clang/lib/Parse/ParseInit.cpp
@@ -35,7 +35,7 @@ bool Parser::MayBeDesignationStart() {
     return true;
 
   case tok::l_square: {  // designator: array-designator
-    if (!PP.getLangOpts().CPlusPlus11)
+    if (!PP.getLangOpts().CPlusPlus)
       return true;
 
     // C++11 lambda expressions and C99 designators can be ambiguous all the
diff --git a/clang/test/Parser/cxx0x-lambda-expressions.cpp b/clang/test/Parser/cxx0x-lambda-expressions.cpp
index 72b315a497c0679..a786a964163e4c4 100644
--- a/clang/test/Parser/cxx0x-lambda-expressions.cpp
+++ b/clang/test/Parser/cxx0x-lambda-expressions.cpp
@@ -1,10 +1,15 @@
-// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify -std=c++11 -Wno-c99-designator %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify -std=c++20 -Wno-c99-designator %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify -std=c++23 -Wno-c99-designator %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx14ext,cxx17ext,cxx20ext,cxx23ext -std=c++03 -Wno-c99-designator %s -Wno-c++11-extensions
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx14ext,cxx17ext,cxx20ext,cxx23ext -std=c++11 -Wno-c99-designator %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx17ext,cxx20ext,cxx23ext          -std=c++14 -Wno-c99-designator %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx20ext,cxx23ext                   -std=c++17 -Wno-c99-designator %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx23ext                            -std=c++20 -Wno-c99-designator %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected                                     -std=c++23 -Wno-c99-designator %s
 
 enum E { e };
 
+#if __cplusplus >= 201103L
 constexpr int id(int n) { return n; }
+#endif
 
 class C {
 
@@ -19,28 +24,25 @@ class C {
     [&,] {}; // expected-error {{expected variable name or 'this' in lambda capture list}}
     [=,] {}; // expected-error {{expected variable name or 'this' in lambda capture list}}
     [] {};
-    [=] (int i) {}; 
-    [&] (int) mutable -> void {}; 
-    [foo,bar] () { return 3; }; 
-    [=,&foo] () {}; 
-    [&,foo] () {}; 
-    [this] () {}; 
+    [=] (int i) {};
+    [&] (int) mutable -> void {};
+    [foo,bar] () { return 3; };
+    [=,&foo] () {};
+    [&,foo] () {};
+    [this] () {};
     [] () -> class C { return C(); };
     [] () -> enum E { return e; };
 
-    [] -> int { return 0; };
-    [] mutable -> int { return 0; };
-#if __cplusplus <= 202002L
-    // expected-warning@-3 {{lambda without a parameter clause is a C++23 extension}}
-    // expected-warning@-3 {{is a C++23 extension}}
-#endif
+    [] -> int { return 0; }; // cxx23ext-warning {{lambda without a parameter clause is a C++23 extension}}
+    [] mutable -> int { return 0; }; // cxx23ext-warning {{is a C++23 extension}}
+
     [](int) -> {}; // PR13652 expected-error {{expected a type}}
     return 1;
   }
 
   void designator_or_lambda() {
-    typedef int T; 
-    const int b = 0; 
+    typedef int T;
+    const int b = 0;
     const int c = 1;
     int d;
     int a1[1] = {[b] (T()) {}}; // expected-error{{no viable conversion from '(lambda}}
@@ -49,19 +51,18 @@ class C {
     int a4[1] = {[&b] = 1 }; // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'const int *'}}
     int a5[3] = { []{return 0;}() };
     int a6[1] = {[this] = 1 }; // expected-error{{integral constant expression must have integral or unscoped enumeration type, not 'C *'}}
-    int a7[1] = {[d(0)] { return d; } ()};
-    int a8[1] = {[d = 0] { return d; } ()};
-    int a10[1] = {[id(0)] { return id; } ()};
-#if __cplusplus <= 201103L
-    // expected-warning@-4{{extension}}
-    // expected-warning@-4{{extension}}
-    // expected-warning@-4{{extension}}
+    int a7[1] = {[d(0)] { return d; } ()}; // cxx14ext-warning {{initialized lambda captures are a C++14 extension}}
+    int a8[1] = {[d = 0] { return d; } ()}; // cxx14ext-warning {{initialized lambda captures are a C++14 extension}}
+#if __cplusplus >= 201103L
+    int a10[1] = {[id(0)] { return id; } ()}; // cxx14ext-warning {{initialized lambda captures are a C++14 extension}}
 #endif
     int a9[1] = {[d = 0] = 1}; // expected-error{{is not an integral constant expression}}
 #if __cplusplus >= 201402L
     // expected-note@-2{{constant expression cannot modify an object that is visible outside that expression}}
 #endif
+#if __cplusplus >= 201103L
     int a11[1] = {[id(0)] = 1};
+#endif
   }
 
   void delete_lambda(int *p) {
@@ -80,43 +81,33 @@ class C {
   // We support init-captures in C++11 as an extension.
   int z;
   void init_capture() {
-    [n(0)] () mutable -> int { return ++n; };
-    [n{0}] { return; };
-    [a([&b = z]{})](){};
-    [n = 0] { return ++n; }; // expected-error {{captured by copy in a non-mutable}}
-    [n = {0}] { return; }; // expected-error {{<initializer_list>}}
-#if __cplusplus <= 201103L
-    // expected-warning@-6{{extension}}
-    // expected-warning@-6{{extension}}
-    // expected-warning@-6{{extension}}
-    // expected-warning@-7{{extension}}
-    // expected-warning@-7{{extension}}
-    // expected-warning@-7{{extension}}
-#endif
+    [n(0)] () mutable -> int { return ++n; }; // cxx14ext-warning    {{initialized lambda captures are a C++14 extension}}
+    [n{0}] { return; };                       // cxx14ext-warning    {{initialized lambda captures are a C++14 extension}}
+    [a([&b = z]{})](){};                      // cxx14ext-warning 2  {{initialized lambda captures are a C++14 extension}}
+    [n = 0] { return ++n; };                  // expected-error      {{captured by copy in a non-mutable}}
+                                              // cxx14ext-warning@-1 {{initialized lambda captures are a C++14 extension}}
+    [n = {0}] { return; };                    // expected-error      {{<initializer_list>}}
+                                              // cxx14ext-warning@-1 {{initialized lambda captures are a C++14 extension}}
 
     int x = 4;
-    auto y = [&r = x, x = x + 1]() -> int {
-#if __cplusplus <= 201103L
-      // expected-warning@-2{{extension}}
-      // expected-warning@-3{{extension}}
-#endif
+    auto y = [&r = x, x = x + 1]() -> int { // cxx14ext-warning 2 {{initialized lambda captures are a C++14 extension}}
       r += 2;
       return x + 2;
     } ();
   }
 
   void attributes() {
-    [] __attribute__((noreturn)){};
-#if __cplusplus <= 202002L
-    // expected-warning@-2 {{is a C++23 extension}}
-#endif
+    [] __attribute__((noreturn)){}; // cxx23ext-warning {{lambda without a parameter clause is a C++23 extension}}
+
     []() [[]]
       mutable {}; // expected-error {{expected body of lambda expression}}
 
     []() [[]] {};
     []() [[]] -> void {};
     []() mutable [[]] -> void {};
+#if __cplusplus >= 201103L
     []() mutable noexcept [[]] -> void {};
+#endif
 
     // Testing GNU-style attributes on lambdas -- the attribute is specified
     // before the mutable specifier instead of after (unlike C++11).
@@ -126,28 +117,18 @@ class C {
 
     // Testing support for P2173 on adding attributes to the declaration
     // rather than the type.
-    [][[]](){};
-#if __cplusplus <= 202002L
-    // expected-warning@-2 {{an attribute specifier sequence in this position is a C++23 extension}}
-#endif
-#if __cplusplus > 201703L
-    []<typename>[[]](){};
-#if __cplusplus <= 202002L
-    // expected-warning@-2 {{an attribute specifier sequence in this position is a C++23 extension}}
-#endif
-#endif
-    [][[]]{};
-#if __cplusplus <= 202002L
-    // expected-warning@-2 {{an attribute specifier sequence in this position is a C++23 extension}}
-#endif
+    [][[]](){}; // cxx23ext-warning {{an attribute specifier sequence in this position is a C++23 extension}}
+
+    []<typename>[[]](){}; // cxx20ext-warning    {{explicit template parameter list for lambdas is a C++20 extension}}
+                          // cxx23ext-warning@-1 {{an attribute specifier sequence in this position is a C++23 extension}}
+
+    [][[]]{}; // cxx23ext-warning {{an attribute specifier sequence in this position is a C++23 extension}}
   }
 
   void missing_parens() {
-    [] mutable {};
-    [] noexcept {};
-#if __cplusplus <= 202002L
-    // expected-warning@-3 {{is a C++23 extension}}
-    // expected-warning@-3 {{is a C++23 extension}}
+    [] mutable {}; // cxx23ext-warning {{is a C++23 extension}}
+#if __cplusplus >= 201103L
+    [] noexcept {}; // cxx23ext-warning {{is a C++23 extension}}
 #endif
   }
 };
@@ -165,10 +146,7 @@ struct A {
 };
 
 struct S {
-  void mf() { A{[*this]{}}; }
-#if __cplusplus < 201703L
-  // expected-warning@-2 {{C++17 extension}}
-#endif
+  void mf() { A(([*this]{})); } // cxx17ext-warning {{'*this' by copy is a C++17 extension}}
 };
 }
 

@llvmbot llvmbot added HLSL HLSL Language Support clang:openmp OpenMP related changes to Clang labels Nov 25, 2023
// NOTE:lambda is not enabled except for hlsl202x.
// expected-error@+2 {{expected expression}}
// expected-warning@+3 {{lambdas are a C++11 extension}}
// expected-error@+2 {{expected body of lambda expression}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this work? I don't know much about HLSL, so I have no idea whether this is expected behaviour or a bug in my patch.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof... So sorry for my delay in responding here. I think there is going to be some challenge in making this work for HLSL, but I'm actually okay with you making that my problem.

@philnik777 philnik777 requested a review from cor3ntin November 25, 2023 14:20
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there an RFC asking the community about exposing lambdas in pre C++11 modes? This is a sufficiently large language extension that we probably should verify if we haven't already. I believe this is a conforming extension (I can't think of a circumstance under which we'd take correct C++98 code and treat it differently), but I don't think it's as simple as you've done here. For example, noexcept is a C++11-ism and can appear on a lambda. constexpr as well, trailing return types, etc. So we'd need a better understanding of what features of lambdas you intend to enable and just how much of the syntax you expect to work in older modes.

The changes should also come with a release note and we need to update Language Extensions Back-ported to Previous Standards in LanguageExtensions.rst to document this.

@philnik777
Copy link
Contributor Author

Was there an RFC asking the community about exposing lambdas in pre C++11 modes? This is a sufficiently large language extension that we probably should verify if we haven't already. I believe this is a conforming extension (I can't think of a circumstance under which we'd take correct C++98 code and treat it differently), but I don't think it's as simple as you've done here. For example, noexcept is a C++11-ism and can appear on a lambda. constexpr as well, trailing return types, etc. So we'd need a better understanding of what features of lambdas you intend to enable and just how much of the syntax you expect to work in older modes.

No, there hasn't been one. I'll write one and address the details you mentioned.

The changes should also come with a release note and we need to update Language Extensions Back-ported to Previous Standards in LanguageExtensions.rst to document this.

I've noticed this too, but didn't have time to update the PR yet.

@shafik
Copy link
Collaborator

shafik commented Nov 30, 2023

Was there an RFC asking the community about exposing lambdas in pre C++11 modes? This is a sufficiently large language extension that we probably should verify if we haven't already. I believe this is a conforming extension (I can't think of a circumstance under which we'd take correct C++98 code and treat it differently), but I don't think it's as simple as you've done here. For example, noexcept is a C++11-ism and can appear on a lambda. constexpr as well, trailing return types, etc. So we'd need a better understanding of what features of lambdas you intend to enable and just how much of the syntax you expect to work in older modes.

Also generic lambdas is probably a big one to think about as well.

@philnik777
Copy link
Contributor Author

@shafik See https://discourse.llvm.org/t/rfc-allow-c-11-lambdas-in-c-03-as-an-extension/75262

Comment on lines 97 to 98
int &ref_j = j; // cxx03-fixme-note {{declared here}}
[] { return ref_j; }; // cxx03-fixme-error {{variable 'ref_j' cannot be implicitly captured in a lambda with no capture-default specified}} cxx03-fixme-note 4 {{capture}} cxx03-fixme-note {{lambda expression begins here}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand this failure. It would be great if someone could point me at the logic for allowing the capture of ref_j.

Copy link
Contributor

@cor3ntin cor3ntin Dec 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref_j is not odr-used because it is a reference whose initializer is itself a
usable in constant expressions (j is static and default initialized).
has static storage duration

Because ref_j is not odr-used, it does not need to be captured.

https://eel.is/c++draft/expr.const#3
https://eel.is/c++draft/expr.const#4.3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK. So this diagnostic is actually correct, since an int& is not usable in a constant expression in C++03. Or am I misunderstanding more?

@philnik777
Copy link
Contributor Author

gentle ping

@philnik777
Copy link
Contributor Author

ping

@shafik shafik requested a review from llvm-beanz February 1, 2024 01:52
@shafik
Copy link
Collaborator

shafik commented Feb 1, 2024

This LGTM but we need @llvm-beanz to review

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like this is all good from the C++ side. I'm fine with this bleeding into HLSL while we figure out what we need to do.

It would be super nice if we can just leave this enabled for HLSL as an extension. A lot of our users want it, but we may need to do some work to make that work within some of the other language constraints.

// NOTE:lambda is not enabled except for hlsl202x.
// expected-error@+2 {{expected expression}}
// expected-warning@+3 {{lambdas are a C++11 extension}}
// expected-error@+2 {{expected body of lambda expression}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof... So sorry for my delay in responding here. I think there is going to be some challenge in making this work for HLSL, but I'm actually okay with you making that my problem.

@philnik777 philnik777 force-pushed the cxx03_lambda_ext branch 3 times, most recently from 73135ec to db504f5 Compare March 9, 2024 00:27
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add a release note letting users know about the new extension in clang/docs/ReleaseNotes.rst, but otherwise, this LGTM!

@philnik777 philnik777 force-pushed the cxx03_lambda_ext branch 2 times, most recently from 9bd0146 to 3caa29d Compare March 17, 2024 20:02
This is a fairly simple extension, but makes the life for people who
have to support C++03 a lot easier. As a nice bonus, this also improves
diagnostics, since lambdas are now properly recognized when parsing
C++03 code.
@philnik777 philnik777 merged commit 2699072 into llvm:main Mar 21, 2024
@LYP951018
Copy link
Contributor

@philnik777 It seems that clang crashes on the test SemaCXX/cxx2a-template-lambdas.cpp https://godbolt.org/z/nnjzKr7n4
Is this expected?

@philnik777
Copy link
Contributor Author

I'm not sure a crash is ever expected, but it's a known issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants