Skip to content
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

[clang] Add separate C++23 extension flag for attrs on lambda #74553

Merged
merged 4 commits into from
Dec 7, 2023

Conversation

usx95
Copy link
Contributor

@usx95 usx95 commented Dec 6, 2023

No description provided.

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

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

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

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-1)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+1-1)
  • (modified) clang/test/SemaCXX/coro-lifetimebound.cpp (+2-8)
  • (modified) clang/test/SemaCXX/coro-return-type-and-wrapper.cpp (+2-7)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index ff028bbbf7426..81443fb16a1f3 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1126,6 +1126,8 @@ def FutureAttrs : DiagGroup<"future-attribute-extensions", [CXX14Attrs,
                                                             CXX17Attrs,
                                                             CXX20Attrs]>;
 
+def CXX23AttrsOnLambda : DiagGroup<"c++23-attrs-on-lambda">;
+
 // A warning group for warnings about using C++11 features as extensions in
 // earlier C++ versions.
 def CXX11 : DiagGroup<"c++11-extensions", [CXX11ExtraSemi, CXX11InlineNamespace,
@@ -1145,7 +1147,7 @@ def CXX20 : DiagGroup<"c++20-extensions", [CXX20Designator, CXX20Attrs]>;
 
 // A warning group for warnings about using C++23 features as extensions in
 // earlier C++ versions.
-def CXX23 : DiagGroup<"c++23-extensions">;
+def CXX23 : DiagGroup<"c++23-extensions", [CXX23AttrsOnLambda]>;
 
 // A warning group for warnings about using C++26 features as extensions in
 // earlier C++ versions.
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 21fe6066d5876..bc7bd7589ef14 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1035,7 +1035,7 @@ def err_capture_default_first : Error<
   "capture default must be first">;
 def ext_decl_attrs_on_lambda : ExtWarn<
   "%select{an attribute specifier sequence|%0}1 in this position "
-  "is a C++23 extension">, InGroup<CXX23>;
+  "is a C++23 extension">, InGroup<CXX23AttrsOnLambda>;
 def ext_lambda_missing_parens : ExtWarn<
   "lambda without a parameter clause is a C++23 extension">,
   InGroup<CXX23>;
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
index d3e2d673ebb3c..ec8997036feb4 100644
--- a/clang/test/SemaCXX/coro-lifetimebound.cpp
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra -Wno-error=unreachable-code -Wno-unused
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra -Wno-error=unreachable-code -Wno-unused -Wno-c++23-attrs-on-lambda
 
 #include "Inputs/std-coroutine.h"
 
@@ -64,14 +64,8 @@ Co<int> bar_coro(const int &b, int c) {
       : bar_coro(0, 1); // expected-warning {{returning address of local temporary object}}
 }
 
-#define CORO_WRAPPER \
-  _Pragma("clang diagnostic push") \
-  _Pragma("clang diagnostic ignored \"-Wc++23-extensions\"") \
-  [[clang::coro_wrapper]] \
-  _Pragma("clang diagnostic pop")
-
 void lambdas() {
-  auto unsafe_lambda = [] CORO_WRAPPER (int b) {
+  auto unsafe_lambda = [] [[clang::coro_wrapper]] (int b) {
     return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
   };
   auto coro_lambda = [] (const int&) -> Co<int> {
diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
index 5f8076f1c782a..d52d1e4cea399 100644
--- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
+++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra -Wno-c++23-attrs-on-lambda
 #include "Inputs/std-coroutine.h"
 
 using std::suspend_always;
@@ -45,11 +45,6 @@ Co<int> non_marked_wrapper(int b) { return foo_coro(b); }
 } // namespace using_decl
 
 namespace lambdas {
-#define CORO_WRAPPER \
-  _Pragma("clang diagnostic push") \
-  _Pragma("clang diagnostic ignored \"-Wc++23-extensions\"") \
-  [[clang::coro_wrapper]] \
-  _Pragma("clang diagnostic pop")
 
 void foo() {
   auto coro_lambda = []() -> Gen<int> {
@@ -59,7 +54,7 @@ void foo() {
   auto not_allowed_wrapper = []() -> Gen<int> {
     return foo_coro(1);
   };
-  auto allowed_wrapper = [] CORO_WRAPPER() -> Gen<int> {
+  auto allowed_wrapper = [] [[clang::coro_wrapper]] () -> Gen<int> {
     return foo_coro(1);
   };
 }

Copy link
Contributor

@ilya-biryukov ilya-biryukov left a comment

Choose a reason for hiding this comment

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

This is useful to us to have attributes on lambdas early in C++20 mode before switching to C++23. I only have a nitpick about naming and happy to approve after it's addressed.

And let's give @AaronBallman a few days to jump in.

clang/include/clang/Basic/DiagnosticGroups.td Outdated Show resolved Hide resolved
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.

Changes LGTM but we should probably document this extension at this point. Can you add this to the table at:

Language Extensions Back-ported to Previous Standards

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.

LGTM, thank you!

@usx95 usx95 merged commit e825cc4 into llvm:main Dec 7, 2023
5 checks passed
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 Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants