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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak abandoned this revision.
ahatanak added a comment.

It looks like a7579b25df78a9f53d62300020d4ae3c4734 
 fixed the 
crash. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann requested changes to this revision.
tahonermann added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaExpr.cpp:19085-19092
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+if (auto *PVD = dyn_cast_or_null(
+ExprEvalContexts.back().ManglingContextDecl))
+  if (PVD->getDeclContext() == DC)

I'm struggling to understand the change. If I'm following correctly, it looks 
like we're trying to capture the variable used in the default argument. If so, 
that seems wrong; I think we shouldn't even be trying to capture a variable for 
such usage.

I jumped into a debugger to poke around a bit. Perhaps the right change is to 
detect the use in a default argument in the code below so that the call to 
`getCapturedDeclRefType()` can be skipped for a non-ODR use.
  /localdisk2/thonerma/llvm-project/clang/lib/Sema/SemaExpr.cpp:
   3265 ExprResult Sema::BuildDeclarationNameExpr(
   3266 const CXXScopeSpec , const DeclarationNameInfo , 
NamedDecl *D,
   3267 NamedDecl *FoundD, const TemplateArgumentListInfo *TemplateArgs,
   3268 bool AcceptInvalidDecl) {
  .
   3326   switch (D->getKind()) {
  .
   3389   case Decl::Var:
   3390   case Decl::VarTemplateSpecialization:
   3391   case Decl::VarTemplatePartialSpecialization:
   3392   case Decl::Decomposition:
   3393   case Decl::OMPCapturedExpr:
   3394 // In C, "extern void blah;" is valid and is an r-value.
   3395 if (!getLangOpts().CPlusPlus && !type.hasQualifiers() &&
   3396 type->isVoidType()) {
   3397   valueKind = VK_PRValue;
   3398   break;
   3399 }
   3400 [[fallthrough]];
   3401
   3402   case Decl::ImplicitParam:
   3403   case Decl::ParmVar: {
   3404 // These are always l-values.
   3405 valueKind = VK_LValue;
   3406 type = type.getNonReferenceType();
   3407
   3408 // FIXME: Does the addition of const really only apply in
   3409 // potentially-evaluated contexts? Since the variable isn't actually
   3410 // captured in an unevaluated context, it seems that the answer is 
no.
   3411 if (!isUnevaluatedContext()) {
   3412   QualType CapturedType = getCapturedDeclRefType(cast(VD), 
Loc);
   3413   if (!CapturedType.isNull())
   3414 type = CapturedType;
   3415 }
   3416
   3417 break;
   3418   }
  .
   3516 }



Comment at: clang/test/SemaCXX/lambda-default-arg.cpp:6
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();

Is the use of `a` in the lambda body intentional? It isn't needed to reproduce 
the assertion failure, so its presence here is a bit confusing from a test 
perspective as it implies that variable capture is somehow related. I suggest 
just returning `true`. Likewise for the other two tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

@tahonermann @cor3ntin have you had a chance to take a look at the updated 
patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

Thanks for the update, @ahatanak. Please note that myself and Corentin are both 
preoccupied with the WG21 meeting this week, so it might be a while before 
either of us is able to take a good look at your update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I agree that pushing an empty scope without initializing any of its members 
seems wrong.

Instead of doing that, the updated patch changes the decl context instead if we 
are trying to capture a variable in a default argument expression of a lambda 
call operator. I think that's okay since the type of the `DeclRefExpr` of the 
variable appearing in the default argument expression shouldn't depend on 
whether it's captured by the lambda, if I understand the standard rules 
correctly (note that when clang crashes, `tryCaptureVariable` is being called 
by `getCapturedDeclRefType`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 495619.
ahatanak added a comment.

Fix indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/lambda-default-arg.cpp


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument 
references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
+
+template  bool test2(float a = 1e-5f) {
+  return [=](int b = sizeof(a)) -> bool {
+return b;
+  }();
+}
+
+bool b2 = test2();
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19082,6 +19082,14 @@
 }
   }
 
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+if (auto *PVD = dyn_cast_or_null(
+ExprEvalContexts.back().ManglingContextDecl))
+  if (PVD->getDeclContext() == DC)
+DC = getLambdaAwareParentOfDeclContext(DC);
 
   // If the variable is declared in the current context, there is no need to
   // capture it.


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
+
+template  bool test2(float a = 1e-5f) {
+  return [=](int b = sizeof(a)) -> bool {
+return b;
+  }();
+}
+
+bool b2 = test2();
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19082,6 +19082,14 @@
 }
   }
 
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+if (auto *PVD = dyn_cast_or_null(
+ExprEvalContexts.back().ManglingContextDecl))
+  if (PVD->getDeclContext() == DC)
+DC = getLambdaAwareParentOfDeclContext(DC);
 
   // If the variable is declared in the current context, there is no need to
   // capture it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 495615.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Instead of pushing an empty lambda scope,  switch to the enclosing context if 
the variable is used in a default argument expression of a lambda call operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/lambda-default-arg.cpp


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument 
references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
+
+template  bool test2(float a = 1e-5f) {
+return [=](int b = sizeof(a)) -> bool {
+  return b;
+}();
+}
+
+bool b2 = test2();
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19082,6 +19082,14 @@
 }
   }
 
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+if (auto *PVD = dyn_cast_or_null(
+ExprEvalContexts.back().ManglingContextDecl))
+  if (PVD->getDeclContext() == DC)
+DC = getLambdaAwareParentOfDeclContext(DC);
 
   // If the variable is declared in the current context, there is no need to
   // capture it.


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
+
+template  bool test2(float a = 1e-5f) {
+return [=](int b = sizeof(a)) -> bool {
+  return b;
+}();
+}
+
+bool b2 = test2();
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19082,6 +19082,14 @@
 }
   }
 
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+if (auto *PVD = dyn_cast_or_null(
+ExprEvalContexts.back().ManglingContextDecl))
+  if (PVD->getDeclContext() == DC)
+DC = getLambdaAwareParentOfDeclContext(DC);
 
   // If the variable is declared in the current context, there is no need to
   // capture it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

My question is, why do we need to mess up with scopes in that way outside of 
parsing  (there are only a couple places where we do that at the moment, and 
they are dummy scopes which only exist to balance some push and pop, afaict 
they serve no other purpose).
As i said, I have no objection to this at all, it clearly fixes the issue and 
we should land it! But the fact we have to do this in the first place *may* be 
a sign that there is a deeper issue, one that we clearly should not try to 
address as part of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> 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 
> (see 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L15435-L15438).

I tried this and the testsuite informed me, in no uncertain terms that, no no, 
that copy is very much still needed. I haven't explored further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 

 (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();

Another example that I think is worth adding:
  template  bool test2(float a = 1e-5f) {
return [=](int b = sizeof(a)) -> bool {
  return b;
}();
  }
  
  bool b2 = test2();


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

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

`PushLambdaScope` creates a new `LambdaScopeInfo` and pushes onto 
`FunctionScopes`.
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/Sema.cpp#L2137

When `Sema::FunctionScopeRAII` goes out of scope, its destructor calls 
`PopFunctionScopeInfo`. 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/Sema.h#L5048

`PopFunctionScopeInfo` then pops the `FunctionScopeInfo` at the top of the 
stack, which is the `LambdaScopeInfo` that was pushed earlier, and 
`PoppedFunctionScopePtr` makes sure the popped scope is deleted.
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/Sema.cpp#L2243


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D143109#4097850 , @ahatanak wrote:

> Doesn't `Sema::FunctionScopeRAII` pop the lambda scope when it goes out of 
> scope?

No, sadly lambdas are handled differently

> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/Sema.h#L5042
>
> I also tried instantiating the default arguments after the call to 
> `BuildLambdaExpr` using `LSICopy` (see 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/TreeTransform.h#L13477),
>  which is the copy of the lambda scope, but I opted for the approach in this 
> patch as using a dummy scope seemed sufficient.

I think the patch is good enough too, just questioning whether we want to add a 
"FIXME" comment for later (It would be good to try to clean up all of that, one 
day - if someone has time)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Doesn't `Sema::FunctionScopeRAII` pop the lambda scope when it goes out of 
scope?

https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/Sema.h#L5042

I also tried instantiating the default arguments after the call to 
`BuildLambdaExpr` using `LSICopy` (see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/TreeTransform.h#L13477),
 which is the copy of the lambda scope, but I opted for the approach in this 
patch as using a dummy scope seemed sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D143109#4097775 , @erichkeane 
wrote:

> @cor3ntin : Mind taking a look here?  You're my lambda expert these days :)

I did something similar to check the requires clause of a lambda and it does 
technically fix the issue, i don't see a more elegant solution, but it does 
sound a bit wrong... (it allocates a dummy function scope that is then kept 
around forever)
Maybe we can add a fixme comment here and add a "remove lambda scope" with an 
raii object for these two cases in a separate PR?

The issue is not surprising though, there are weird order of operations in how 
parameters and functions and their context are handled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: cor3ntin.
erichkeane added a comment.

@cor3ntin : Mind taking a look here?  You're my lambda expert these days :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: tahonermann, erichkeane, aaron.ballman, shafik, 
Bigcheese, arphaman.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

This is needed to fix https://github.com/llvm/llvm-project/issues/60155.

An assertion in `tryCaptureVariable` fails because `SubstDefaultArgument` 
pushes the context for the lambda's function operator, but the LambdaScopeInfo 
isn't being pushed to the stack.

It appears that the assertion started to fail in 
4409a83c293537e22da046b54e9f69454cdd3dca 
, which 
delays instantiation of default arguments. By the time the default arguments 
are instantiated, which happens after `inherited::TransformLambdaExpr` is 
called, the lambda scope has already been popped.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143109

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/lambda-default-arg.cpp


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument 
references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1348,6 +1348,13 @@
 Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
 // FIXME: Obtain the source location for the '=' token.
 SourceLocation EqualLoc = UninstExpr->getBeginLoc();
+
+// 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 (SemaRef.SubstDefaultArgument(EqualLoc, PVD, TemplateArgs)) {
   // If substitution fails, the default argument is set to a
   // RecoveryExpr that wraps the uninstantiated default argument so


Index: clang/test/SemaCXX/lambda-default-arg.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-default-arg.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only %s
+
+template  bool test0() {
+  constexpr float a = 1e-5f;
+  return [=](float b = a) -> bool {
+return a < 0;
+  }();
+}
+
+bool b0 = test0();
+
+template  bool test1() {
+  float a = 1e-5f;
+  return [=](float b = a) -> bool { // expected-error {{default argument references local variable 'a'}}
+return a < 0;
+  }();
+}
+
+bool b1 = test1();
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1348,6 +1348,13 @@
 Expr *UninstExpr = PVD->getUninstantiatedDefaultArg();
 // FIXME: Obtain the source location for the '=' token.
 SourceLocation EqualLoc = UninstExpr->getBeginLoc();
+
+// 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 (SemaRef.SubstDefaultArgument(EqualLoc, PVD, TemplateArgs)) {
   // If substitution fails, the default argument is set to a
   // RecoveryExpr that wraps the uninstantiated default argument so
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits