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

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://review

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

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, sw

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

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 ___

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

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

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

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 thin

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

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-d

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

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 LLV

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

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

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

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/blo

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

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/cl

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

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::FunctionScop

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

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/inclu

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

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`

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

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 do

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

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 ___

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

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/issue