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] Fix a DeclContext mismatch when parsing nested lambda parameters #112177

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Oct 14, 2024

When parsing its function parameters, we don't change the CurContext to the lambda's function declaration. However, CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures() has not yet adapted to such behavior when nested lambdas come into play. Consider the following case,

struct Foo {};

template <int, Foo f> struct Arr {};

constexpr void foo() {
  constexpr Foo F;
  [&]<int I>() {
     [&](Arr<I, F>) {};
   }.template operator()<42>();
}

As per [basic.def.odr]p5.2, the use of F constitutes an ODR-use. And per [basic.def.odr]p10, F should be ODR-usable in that interleaving scope.

We failed to accept the case because the call to tryCaptureVariable() in getStackIndexOfNearestEnclosingCaptureCapableLambda() suggested that F is needlessly captureable. That was due to a missed handling for AfterParameterList in FunctionScopeIndexToStopAt, where it still presumed DC and LSI matched.

Fixes #47400
Fixes #90896

@zyn0217 zyn0217 marked this pull request as ready for review October 14, 2024 11:52
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

When parsing its function parameters, we don't change the CurContext to the lambda's function declaration. However, CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures() has not yet adapted to such behavior when nested lambdas come into play. Consider the following case,

struct Foo {};

template &lt;int, Foo f&gt; struct Arr {};

constexpr void foo() {
  constexpr Foo F;
  [&amp;]&lt;int I&gt;() {
     [&amp;](Arr&lt;I, F&gt;) {};
   }.template operator()&lt;42&gt;();
}

As per [basic.def.odr]p5.2, the use of F constitutes an ODR-use. And per [basic.def.odr]p10, F should be ODR-usable in that interleaving scope.

We failed to accept the case because the call to tryCaptureVariable() in getStackIndexOfNearestEnclosingCaptureCapableLambda() suggested that F is needlessly captureable. That was due to a missed handling for AfterParameterList in FunctionScopeIndexToStopAt, where it still presumed DC and LSI matched.

Fixes #47400
Fixes #90896


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

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+10)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-1)
  • (modified) clang/test/SemaCXX/lambda-capture-type-deduction.cpp (+23)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8d02cc3eae9fd9..3d6da43cf02f69 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,8 @@ Bug Fixes to C++ Support
 - Clang now uses the correct set of template argument lists when comparing the constraints of
   out-of-line definitions and member templates explicitly specialized for a given implicit instantiation of
   a class template. (#GH102320)
+- Fixed a bug in lambda captures where ``constexpr`` class-type objects were not properly considered ODR-used in
+  certain situations. (#GH47400), (#GH90896)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index f930a21ea870ec..0075ec86823920 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18843,7 +18843,17 @@ bool Sema::tryCaptureVariable(
   // We need to sync up the Declaration Context with the
   // FunctionScopeIndexToStopAt
   if (FunctionScopeIndexToStopAt) {
+    assert(!FunctionScopes.empty() && "No function scopes to stop at?");
     unsigned FSIndex = FunctionScopes.size() - 1;
+    // When we're parsing the lambda parameter list, the current DeclContext is
+    // NOT the lambda but its parent. So move away the current LSI before
+    // aligning DC and FunctionScopeIndexToStopAt.
+    if (auto *LSI = dyn_cast<LambdaScopeInfo>(FunctionScopes[FSIndex]);
+        LSI && !LSI->AfterParameterList)
+      --FSIndex;
+    assert(MaxFunctionScopesIndex <= FSIndex &&
+           "FunctionScopeIndexToStopAt should be no larger than FSIndex into "
+           "FunctionScopes.");
     while (FSIndex != MaxFunctionScopesIndex) {
       DC = getLambdaAwareParentOfDeclContext(DC);
       --FSIndex;
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d490452e91c3bb..09ab83dcf21c22 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8679,7 +8679,7 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
   while (isa_and_nonnull<CapturedDecl>(DC))
     DC = DC->getParent();
   assert(
-      CurrentLSI->CallOperator == DC &&
+      (CurrentLSI->CallOperator == DC || !CurrentLSI->AfterParameterList) &&
       "The current call operator must be synchronized with Sema's CurContext");
 #endif // NDEBUG
 
diff --git a/clang/test/SemaCXX/lambda-capture-type-deduction.cpp b/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
index 7bf36a6a9cab73..253d0f429abbed 100644
--- a/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
+++ b/clang/test/SemaCXX/lambda-capture-type-deduction.cpp
@@ -297,3 +297,26 @@ void __trans_tmp_1() {
 }
 
 }
+
+namespace GH47400 {
+
+struct Foo {};
+
+template <int, Foo> struct Arr {};
+
+template <int> struct S {};
+
+constexpr bool foo() {
+  constexpr Foo f;
+  [&]<int is>() {
+    [&](Arr<is, f>) {}({}); // f constitutes an ODR-use
+  }.template operator()<42>();
+
+  constexpr int C = 1;
+  [] {
+    [](S<C>) { }({}); // ... while C doesn't
+  }();
+  return true;
+}
+
+} // namespace GH47400

@zyn0217 zyn0217 merged commit 3733b0c into llvm:main Oct 15, 2024
9 checks passed
@zyn0217 zyn0217 deleted the issue-47400 branch October 15, 2024 04:52
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…ers (llvm#112177)

When parsing its function parameters, we don't change the CurContext to
the lambda's function declaration. However,
CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures() has not
yet adapted to such behavior when nested lambdas come into play.
Consider the following case,

    struct Foo {};

    template <int, Foo f> struct Arr {};

    constexpr void foo() {
      constexpr Foo F;
      [&]<int I>() {
         [&](Arr<I, F>) {};
       }.template operator()<42>();
    }

As per [basic.def.odr]p5.2, the use of F constitutes an ODR-use. And
per [basic.def.odr]p10, F should be ODR-usable in that interleaving
scope.

We failed to accept the case because the call to tryCaptureVariable()
in getStackIndexOfNearestEnclosingCaptureCapableLambda() suggested
that F is needlessly captureable. That was due to a missed handling
for AfterParameterList in FunctionScopeIndexToStopAt, where it still
presumed DC and LSI matched.

Fixes llvm#47400
Fixes llvm#90896
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
…ers (llvm#112177)

When parsing its function parameters, we don't change the CurContext to
the lambda's function declaration. However,
CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures() has not
yet adapted to such behavior when nested lambdas come into play.
Consider the following case,

    struct Foo {};

    template <int, Foo f> struct Arr {};

    constexpr void foo() {
      constexpr Foo F;
      [&]<int I>() {
         [&](Arr<I, F>) {};
       }.template operator()<42>();
    }

As per [basic.def.odr]p5.2, the use of F constitutes an ODR-use. And
per [basic.def.odr]p10, F should be ODR-usable in that interleaving
scope.

We failed to accept the case because the call to tryCaptureVariable()
in getStackIndexOfNearestEnclosingCaptureCapableLambda() suggested
that F is needlessly captureable. That was due to a missed handling
for AfterParameterList in FunctionScopeIndexToStopAt, where it still
presumed DC and LSI matched.

Fixes llvm#47400
Fixes llvm#90896
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
…ers (llvm#112177)

When parsing its function parameters, we don't change the CurContext to
the lambda's function declaration. However,
CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures() has not
yet adapted to such behavior when nested lambdas come into play.
Consider the following case,

    struct Foo {};

    template <int, Foo f> struct Arr {};

    constexpr void foo() {
      constexpr Foo F;
      [&]<int I>() {
         [&](Arr<I, F>) {};
       }.template operator()<42>();
    }

As per [basic.def.odr]p5.2, the use of F constitutes an ODR-use. And
per [basic.def.odr]p10, F should be ODR-usable in that interleaving
scope.

We failed to accept the case because the call to tryCaptureVariable()
in getStackIndexOfNearestEnclosingCaptureCapableLambda() suggested
that F is needlessly captureable. That was due to a missed handling
for AfterParameterList in FunctionScopeIndexToStopAt, where it still
presumed DC and LSI matched.

Fixes llvm#47400
Fixes llvm#90896
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
4 participants