[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

BertalanD added a comment. FYI, we found a crash related to this patch: https://github.com/llvm/llvm-project/issues/67260 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane added a comment. In D159126#4642023 , @erichkeane wrote: > In D159126#4642010 , @thakis wrote: > >> Looks like the test is missing a RUN line and hence makes lit fail: >>

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane added a comment. In D159126#4642010 , @thakis wrote: > Looks like the test is missing a RUN line and hence makes lit fail: > http://45.33.8.238/linux/117631/step_7.txt > > Please take a look and revert for now if it takes a while to fix.

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

thakis added a comment. Looks like the test is missing a RUN line and hence makes lit fail: http://45.33.8.238/linux/117631/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3ed9e9e3ace6: [Clang] Add captures to the instantiation scope of lambda call operators (authored by cor3ntin). Repository: rG LLVM Github

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added a comment. @erichkeane Ok, I added a test! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 ___ cfe-commits mailing list

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin updated this revision to Diff 556209. cor3ntin added a comment. Add the test case that caused a build failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 Files: clang/docs/ReleaseNotes.rst

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane added a comment. In D159126#4640484 , @cor3ntin wrote: > Fix libc++ compile > > @erichkeane I struggle to reduce further, do you think it's worth adding this > inscrutable > test case somewhere? There's definitely value to putting it in

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin updated this revision to Diff 556115. cor3ntin added a comment. Fix libc++ compile @erichkeane I struggle to reduce further, do you think it's worth adding this inscrutable test case somewhere? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added a comment. @abrachet Thanks, i've reverted the patch while i investigate a fix I managed to reduced it to template concept cs = requires(b bj) { bj.begin(); }; struct { template requires cs auto operator()(b &&) {} } begin; template concept cu =

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

abrachet added a comment. I'm seeing libcxx failures after this patch. See https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8770673168839783889/overview # .---command stderr # |

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeaf725bc9371: [Clang] Add captures to the instantiation scope of lambda call operators (authored by cor3ntin). Repository: rG LLVM Github

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:2263 +LocalInstantiationScope ) +: FunctionScope(SemasRef) { + if (!isLambdaCallOperator(FD)) { cor3ntin wrote: > erichkeane wrote: > > cor3ntin wrote: > > > erichkeane

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:2263 +LocalInstantiationScope ) +: FunctionScope(SemasRef) { + if (!isLambdaCallOperator(FD)) { erichkeane wrote: > cor3ntin wrote: > > erichkeane wrote: > > > Wonder if

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin updated this revision to Diff 556068. cor3ntin added a comment. Inherit from FunctionScopeRAII as per @erichkeane Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 Files:

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaLambda.cpp:2263 +LocalInstantiationScope ) +: FunctionScope(SemasRef) { + if (!isLambdaCallOperator(FD)) {

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:2263 +LocalInstantiationScope ) +: FunctionScope(SemasRef) { + if (!isLambdaCallOperator(FD)) { erichkeane wrote: > Wonder if `LambdaScopeForCallOperatorInstantiationRAII`

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

erichkeane added a comment. 1 suggestion, else LGTM. Comment at: clang/lib/Sema/SemaLambda.cpp:2263 +LocalInstantiationScope ) +: FunctionScope(SemasRef) { + if (!isLambdaCallOperator(FD)) { Wonder if `LambdaScopeForCallOperatorInstantiationRAII`

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added a comment. @erichkeane @aaron.ballman This now pass the tests and is ready for review :) Thanks to @beanz for the quick turnaround on the HLSL issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin updated this revision to Diff 555974. cor3ntin added a comment. Rebase and cleanups Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 Files: clang/docs/ReleaseNotes.rst

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin updated this revision to Diff 555973. cor3ntin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaConcept.cpp

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

beanz added a comment. Yea, the gist of it is that in HLSL `this` is a reference not a pointer, which means the `CXXThisExpr` is always an LValue. I think the right fix for this is to cleanup the `CXXThisExpr` creation code and create a `CXXThisExpr::Create` method like the other AST nodes.

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added a comment. @beanz Oh wow, excellent, i did not expect a solution so fast! I'm glad to know there may indeed be an issue with HLSL rather than this patch because I'm not sure I could have come up with a different fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

beanz added a comment. @cor3ntin, I know what the problem is and I think I can put up a review for a fix tonight or (more likely) tomorrow. Is that okay? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

beanz added a comment. I'll apply this patch and debug the issue today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159126/new/ https://reviews.llvm.org/D159126 ___ cfe-commits mailing list

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

aaron.ballman added a subscriber: bogner. aaron.ballman added a comment. In D159126#4625911 , @cor3ntin wrote: > @beanz The changes to TreeTransform.h cause a lot of HLSL test failures > similar to this one: > > clang: >

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin added a subscriber: beanz. cor3ntin added a comment. @beanz The changes to TreeTransform.h cause a lot of HLSL test failures similar to this one: clang: /home/cor3ntin/dev/compilers/LLVM/llvm-project/clang/lib/AST/ExprClassification.cpp:57: Cl

[PATCH] D159126: [Clang] Add captures to the instantiation scope of lambda call operators

cor3ntin created this revision. Herald added subscribers: ChuanqiXu, Anastasia. Herald added a project: All. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Like concepts checking, a trailing return type of a lambda in a dependent