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] Don't add top-level const qualifiers to captured function types #118050

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Nov 29, 2024

This aligns with the logic in TreeTransform::RebuildQualifiedType() where we refrain from adding const qualifiers to function types. Previously, we seemed to overlook this edge case when copy-capturing a variable that is of function type within a const-qualified lambda.

This issue also reveals other related problems as in incorrect type printout and a suspicious implementation in DeduceTemplateArguments. I decide to leave them in follow-up work.

Fixes #84961

@zyn0217 zyn0217 marked this pull request as ready for review November 29, 2024 06:39
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2024

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

This aligns with the logic in TreeTransform::RebuildQualifiedType() where we refrain from adding const qualifiers to function types. Previously, we seemed to overlook this edge case when copy-capturing a variable that is of function type within a const-qualified lambda.

This issue also reveals other related problems as in incorrect type printout and a suspicious implementation in DeduceTemplateArguments. I decide to leave them in follow-up work.

Fixes #84961


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+9-2)
  • (modified) clang/test/SemaCXX/lambda-capture-type-deduction.cpp (+14)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8bd06fadfdc984..553856f3060bc1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -712,6 +712,7 @@ Bug Fixes to C++ Support
 - Name independent data members were not correctly initialized from default member initializers. (#GH114069)
 - Fixed expression transformation for ``[[assume(...)]]``, allowing using pack indexing expressions within the
   assumption if they also occur inside of a dependent lambda. (#GH114787)
+- Lambdas now capture function types without considering top-level const qualifiers. (#GH84961)
 - Clang now uses valid deduced type locations when diagnosing functions with trailing return type
   missing placeholder return type. (#GH78694)
 
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c7472ce92703b..04b713a91dedfa 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18479,7 +18479,10 @@ static bool isVariableAlreadyCapturedInScopeInfo(CapturingScopeInfo *CSI,
     // are mutable in the sense that user can change their value - they are
     // private instances of the captured declarations.
     const Capture &Cap = CSI->getCapture(Var);
-    if (Cap.isCopyCapture() &&
+    // C++ [dcl.fct]p7:
+    //   [When] adding cv-qualifications on top of the function type [...] the
+    //   cv-qualifiers are ignored.
+    if (Cap.isCopyCapture() && !DeclRefType->isFunctionType() &&
         !(isa<LambdaScopeInfo>(CSI) &&
           !cast<LambdaScopeInfo>(CSI)->lambdaCaptureShouldBeConst()) &&
         !(isa<CapturedRegionScopeInfo>(CSI) &&
@@ -18789,7 +18792,11 @@ static bool captureInLambda(LambdaScopeInfo *LSI, ValueDecl *Var,
     //   parameter-declaration-clause is not followed by mutable.
     DeclRefType = CaptureType.getNonReferenceType();
     bool Const = LSI->lambdaCaptureShouldBeConst();
-    if (Const && !CaptureType->isReferenceType())
+    // C++ [dcl.fct]p7:
+    //   [When] adding cv-qualifications on top of the function type [...] the
+    //   cv-qualifiers are ignored.
+    if (Const && !CaptureType->isReferenceType() &&
+        !DeclRefType->isFunctionType())
       DeclRefType.addConst();
   }
 
diff --git a/clang/test/SemaCXX/lambda-capture-type-deduction.cpp b/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
index a86f3018989927..ba7ab34f943be1 100644
--- a/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
+++ b/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
@@ -319,3 +319,17 @@ constexpr void foo() {
 }
 
 } // namespace GH47400
+
+namespace GH84961 {
+
+template <typename T> void g(const T &t) {}
+
+template <typename T> void f(const T &t) {
+  [t] { g(t); }();
+}
+
+void h() {
+  f(h);
+}
+
+} // namespace GH84961

@zyn0217 zyn0217 merged commit 154c7c0 into llvm:main Dec 4, 2024
9 checks passed
@zyn0217 zyn0217 deleted the const-qualifiers-lambda-84961 branch December 4, 2024 07:31
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
3 participants