[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> I'm still don't understand what the problem is about cleaning up the lambda scope.

Me either. It looks to me like the pushed lambda scope will get properly cleaned up.

Incidentally, it looks like that LSI copy (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/TreeTransform.h#L13469-L13478) has not been needed since commit bf5fe2dbba0899bee4323f5eaa075acc43a18e2e <https://github.com/llvm/llvm-project/commit/bf5fe2dbba0899bee4323f5eaa075acc43a18e2e> (see https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L15435-L15438).



================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1352-1356
+        // Push an empty lambda scope. This is needed as SubstDefaultArgument
+        // pushes the context for the lambda's function call operator and we
+        // need to keep the contexts and the function scopes in sync.
+        getSema().PushLambdaScope();
+        Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
----------------
If I'm understanding the issue correctly, the problem is that the lambda scope was created within the call to `inherited::TransformLambdaExpr(E)` above, but then popped upon return (and the copy created to outlive the call to `Sema::ActOnFinishFunctionBody()` destructed). A better solution then would be to modify `TreeTransform::TransformLambdaExpr()` to include this loop and dispatch transformation of the default argument via `getDerived().transformLambdaDefaultArgument()` (or similar). That seems like a straight forward refactor that might be useful elsewhere.


================
Comment at: clang/test/SemaCXX/lambda-default-arg.cpp:19
+
+bool b1 = test1<int>();
----------------
Another example that I think is worth adding:
  template <class T> bool test2(float a = 1e-5f) {
    return [=](int b = sizeof(a)) -> bool {
      return b;
    }();
  }
  
  bool b2 = test2<int>();


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143109/new/

https://reviews.llvm.org/D143109