[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

mizvekov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if (auto

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

mizvekov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if (auto

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Daisy's bug ended up being more complicated than I expected... there is a lot to unpack here. In the meantime, I've captured the bug here: https://github.com/llvm/llvm-project/issues/58368 and will continue looking at it. Repository: rG LLVM Github Monorepo

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#382 , @erichkeane wrote: > In D126907#3854983 , @cor3ntin > wrote: > >> @erichkeane I was looking at Daisy meta programming shenanigans and found >> this crashes

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3854983 , @cor3ntin wrote: > @erichkeane I was looking at Daisy meta programming shenanigans and found > this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your > work, probably? It used to work

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

cor3ntin added a comment. @erichkeane I was looking at Daisy meta programming shenanigans and found this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your work, probably? It used to work in Clang 15. I thought you might want to know! Repository: rG LLVM Github Monorepo

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a subscriber: Uthkarsh. erichkeane added a comment. In D126907#3852975 , @BertalanD wrote: > (I know this is a very contrived example with `void operator!=`, but that is > what CVise spat out, and the actual failures are related to

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

BertalanD added a comment. (I know this is a very contrived example with `void operator!=`, but that is what CVise spat out, and the actual failures are related to comparison operators too) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3852909 , @BertalanD wrote: > I tried out D135772 , and our build got > significantly farther than before! I unfortunately discovered another piece > of code that pre

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

BertalanD added a comment. I tried out D135772 , and our build got significantly farther than before! I unfortunately discovered another piece of code that pre babdef27c503c0bbbcc017e9f88affddda90ea4e

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3849583 , @erichkeane wrote: > In D126907#3846591 , @erichkeane > wrote: > >> In D126907#3844788 , @BertalanD >> wrote: >> >>>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

BertalanD added a comment. In D126907#3849583 , @erichkeane wrote: > In D126907#3846591 , @erichkeane > wrote: > >> In D126907#3844788 , @BertalanD >> wrote: >> >>> Hi

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3846591 , @erichkeane wrote: > In D126907#3844788 , @BertalanD > wrote: > >> Hi @erichkeane, >> >> This change broke compilation of this program >>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3844788 , @BertalanD wrote: > Hi @erichkeane, > > This change broke compilation of this program > (https://godbolt.org/z/KrWGvcf8h; reduced from > https://github.com/SerenityOS/ladybird): > > template >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

BertalanD added a comment. Hi @erichkeane, This change broke compilation of this program (https://godbolt.org/z/KrWGvcf8h; reduced from https://github.com/SerenityOS/ladybird): template constexpr bool IsSame = false; template constexpr bool IsSame = true; template struct Foo

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3807707 , @wlei wrote: > Tested this and confirmed the issue I reported is gone, thanks! Thank you all for the quick responses! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

wlei added a comment. Tested this and confirmed the issue I reported is gone, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. Tested with our internal workloads and things look fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3806760 , @Mordante wrote: > In D126907#3805606 , @erichkeane > wrote: > >> Now fully runs libcxx modules configuration as well, and passes the >> minimization examples

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a comment. In D126907#3805606 , @erichkeane wrote: > Now fully runs libcxx modules configuration as well, and passes the > minimization examples from @wlei . > > @wlei and anyone else: can you please try running this against your >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

wlei added a comment. In D126907#3805606 , @erichkeane wrote: > Now fully runs libcxx modules configuration as well, and passes the > minimization examples from @wlei . > > @wlei and anyone else: can you please try running this against your >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Failure I noticed was NOT a regression: https://godbolt.org/z/MnvqP88vv CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 ___ cfe-commits mailing list

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3805640 , @erichkeane wrote: > I spoke too soon, I found another crash that came out of @wlei s repro from > last time. Actually, what I found was a reduction that had gone haywire before and generated invalid

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. I spoke too soon, I found another crash that came out of @wlei s repro from last time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 ___ cfe-commits mailing list

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane reopened this revision. erichkeane added a comment. This revision is now accepted and ready to land. I have a replacement patch that should fix all of the bugs I'm aware of. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3777088 , @ldionne wrote: > In D126907#3746477 , @Mordante > wrote: > >> Unfortunately there are a lot of different options and combination of >> options to test libc++.

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ldionne added a comment. In D126907#3746477 , @Mordante wrote: > Unfortunately there are a lot of different options and combination of options > to test libc++. > So it's indeed not possible to test all options with once check-cxx > invocation. This

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a comment. In D126907#3740536 , @erichkeane wrote: > For example, this enable-modules flag isn't covered by my local check-cxx, > which I went out of my way to run on this patch locally. Unfortunately there are a lot of different

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3740249 , @erichkeane wrote: > FWIW, while I wasn't able to reproduce the problem that @Mordante reported, I > found that the test suite as given has a large number of asserts in debug > mode while trying to

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. I get that libcxx doesn't run on precommit CI (which is unfortunate for those of us modifying the CFE), but it becomes difficult to run locally, when check-all (requires check-runtimes or check-cxx) don't cover it, and now even check-cxx doesn't cover it. >> What

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a comment. In D126907#3739923 , @aaron.ballman wrote: > In D126907#3739750 , @Mordante > wrote: > >> In D126907#3738383 , @erichkeane >> wrote: >> >>>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. FWIW, while I wasn't able to reproduce the problem that @Mordante reported, I found that the test suite as given has a large number of asserts in debug mode while trying to compare parameter-mappings during constraint normalization(assert is `clang-15:

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3739750 , @Mordante wrote: > In D126907#3738383 , @erichkeane > wrote: > >> There was a test I dealt with previously where a ton of the header files >> were run with

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

aaron.ballman added a comment. In D126907#3739750 , @Mordante wrote: > In D126907#3738383 , @erichkeane > wrote: > >> There was a test I dealt with previously where a ton of the header files >> were run with

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a subscriber: aaron.ballman. Mordante added a comment. In D126907#3738383 , @erichkeane wrote: > There was a test I dealt with previously where a ton of the header files were > run with modules enabled (and an auto generated files), so

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a subscriber: ldionne. erichkeane added a comment. In D126907#3737986 , @Mordante wrote: > In D126907#3737641 , @erichkeane > wrote: > >> In D126907#3737375

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a comment. In D126907#3737641 , @erichkeane wrote: > In D126907#3737375 , @Mordante > wrote: > >> This change breaks libc++'s modular build see build >>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3737375 , @Mordante wrote: > This change breaks libc++'s modular build see build > https://buildkite.com/llvm-project/libcxx-ci/builds/12991 > Reverting this commit on top of yesterday's build (before your revert)

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

Mordante added a comment. This change breaks libc++'s modular build see build https://buildkite.com/llvm-project/libcxx-ci/builds/12991 Reverting this commit on top of yesterday's build (before your revert) fixes the issue. libc++'s modular build uses Clang's pre-C++20 modules. It can be

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd483730d8c3f: Re-apply Deferred Concept Instantiation Implementation (authored by erichkeane). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 453055. erichkeane marked 5 inline comments as done. erichkeane added a comment. Respond/fix all of the things @ChuanqiXu mentioned. Intend to commit early tomorrow based on latest feedback unless others have concerns. CHANGES SINCE LAST ACTION

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane marked 8 inline comments as done. erichkeane added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3642 + // template, for the purposes of [temp.friend] p9. + bool ConstraintExpressionDependsOnEnclosingTemplate(unsigned TemplateDepth, +

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. I am not sure if I don't miss any points. But if it is OK to run libc++/libstdc++, I think it should be good to land this. Comment at: clang/include/clang/Sema/Sema.h:3642 + // template, for the purposes of [temp.friend] p9. + bool

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. SO, I charted the differences in runtimes, and my patch is BETTER time-wise in release mode, but WORSE with asserts enabled. I don't know of any expensive assert that I added. SO, if everyone could review this, I'd love to get this committed to see if it beats the

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. I think I've proved to myself that there IS a compile-time regression, but it is reasonably minor. I suspect a lot of it is going to be the 'friend' comparisons not caching, which I might look into doing. CHANGES SINCE LAST ACTION

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3715436 , @ChuanqiXu wrote: > In D126907#3712363 , @erichkeane > wrote: > >> In D126907#3711956 , @ChuanqiXu >> wrote: >> >>> In

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3712363 , @erichkeane wrote: > In D126907#3711956 , @ChuanqiXu > wrote: > >> In D126907#3710656 , @erichkeane >> wrote: >> >>>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3711956 , @ChuanqiXu wrote: > In D126907#3710656 , @erichkeane > wrote: > >> Ok, fixed the test failure in clang, AND it managed to fix >> all the failures in libcxx! >>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3711956 , @ChuanqiXu wrote: > In D126907#3710656 , @erichkeane > wrote: > >> Ok, fixed the test failure in clang, AND it managed to fix >> all the failures in libcxx! >>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3710656 , @erichkeane wrote: > Ok, fixed the test failure in clang, AND it managed to fix > all the failures in libcxx! > > HOWEVER, it appears that libcxx/test/libcxx/modules_include.sh.cpp > is now hanging? > > I

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 451232. erichkeane added a comment. Ok, fixed the test failure in clang, AND it managed to fix all the failures in libcxx! HOWEVER, it appears that libcxx/test/libcxx/modules_include.sh.cpp is now hanging? I don't know much about the modules

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 450857. erichkeane added a comment. I pulled out a good amount of this patch as NFC, which should shrink the patch. I have a 'new approach' that I think will work, which is to suppress the constraint evaluation the same way we did with the requires

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3667265 , @ChuanqiXu wrote: > In D126907#3665900 , @erichkeane > wrote: > >> The more I look at this... the more I question my approach. I now am >> definitely sure we

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3665900 , @erichkeane wrote: > The more I look at this... the more I question my approach. I now am > definitely sure we won't make Clang15, and hope that I can figure something > better out for 16 :/ I feel

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. The more I look at this... the more I question my approach. I now am definitely sure we won't make Clang15, and hope that I can figure something better out for 16 :/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Hmm... I think my approach was slightly off... I have to spend more time on this, but thanks for the review! I hope I'm learning each time we go again :/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Ah... I remember now, and I think that solution isn't quite right. The idea is that we want to defer constraint evaluation when the constraint is on a function decl/template decl. The PROBLEM I need to solve is when it is in a body of a struct or function.

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:963-964 + Entity(Entity), + EvaluatingAConstraint(EvaluatingConstraint || +!SemaRef.CurContext->isDependentContext()) {}

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:963-964 + Entity(Entity), + EvaluatingAConstraint(EvaluatingConstraint || +!SemaRef.CurContext->isDependentContext()) {}

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:963-964 + Entity(Entity), + EvaluatingAConstraint(EvaluatingConstraint || +!SemaRef.CurContext->isDependentContext()) {}

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3645198 , @erichkeane wrote: > In D126907#3644127 , @ChuanqiXu > wrote: > >> In D126907#3642530 , @erichkeane >> wrote: >> >>>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3644127 , @ChuanqiXu wrote: > In D126907#3642530 , @erichkeane > wrote: > >> This version passes check-runtimes, so libc++ is fine, and it passes >> everything I have

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3642530 , @erichkeane wrote: > This version passes check-runtimes, so libc++ is fine, and it passes > everything I have available. @ChuanqiXu : Would you be able to do 1 more run > over this to make sure it won't

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3636830 , @erichkeane wrote: > SO I've been playing with this for a while and all the libcxx issues. I THINK > we really need to just bit the bullet and figure out how to correctly re-add > things to the

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. SO I've been playing with this for a while and all the libcxx issues. I THINK we really need to just bit the bullet and figure out how to correctly re-add things to the instantiation scope (after making the CheckFunctionConstraints LocalInstantiationScope 'false'

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Welp, fixed THIS problem but lead to another issue in libc++ that I'm still running down. Stand by all :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

vitalybuka reopened this revision. vitalybuka added a comment. This revision is now accepted and ready to land. In D126907#3623377 , @vitalybuka wrote: > FYI. The same on a different bot >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Crash that required the revert : https://reviews.llvm.org/rGbefa8cf087dbb8159a4d9dc8fa4d6748d6d5049a reduces to : template concept sentinel_for = requires(_Ip __i) { __i++; }; template concept bidirectional_iterator =

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3625600 , @vitalybuka wrote: > @erichkeane can you please make sure that committed revisions in git have > "Differential Revision: ". Usually it's added by "arc" when you upload > review. > Also it would be

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

vitalybuka added a comment. @erichkeane can you please make sure that committed revisions in git have "Differential Revision: ". Usually it's added by "arc" when you upload review. Also it would be nice if reapply have the same link. Regarding reproducer, you can follow

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. @shafik was able to help me figure out how to repro, and I think I've about fixed this, so I'll try another commit soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Herald added a reviewer: ributzka. I tried the ninja stage2-check-all on my local build with this patch applied and don't reproduce the issue. Is there any way someone could get me the preprocessed source that this crashed on off the servers or something?

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

vitalybuka added a comment. > Yikes! Thanks for the revert. I didn't see the email from the bot until > just about 2 minutes before you reverted. I'll see if I can reproduce. FYI. The same on a different bot https://lab.llvm.org/buildbot/#/builders/5/builds/25486/steps/19/logs/stdio

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3623120 , @JDevlieghere wrote: > This change triggers an assertion when building an LLDB test case: > > UNREACHABLE executed at >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

JDevlieghere added a comment. This change triggers an assertion when building an LLDB test case: UNREACHABLE executed at /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/lib/Sema/SemaTemplate.cpp:1726! Assertion failed: (InstantiatingSpecializations.empty() && "failed to

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f207439521d: Deferred Concept Instantiation Implementation (authored by erichkeane). Herald added a project: clang. Repository: rG LLVM Github

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/include/clang/AST/Decl.h:1944-1948 + /// For non-templates this value will be NULL, unless this non-template + /// function declaration was declared directly inside of a function template, + /// in which case this will have

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. In D126907#3619270 , @erichkeane wrote: > All but the 1 comment from @ChuanqiXu fixed, not sure what to do about the > 'info'. LGTM. But the

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 441020. erichkeane marked 2 inline comments as done. erichkeane added a comment. All but the 1 comment from @ChuanqiXu fixed, not sure what to do about the 'info'. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane marked 9 inline comments as done. erichkeane added a comment. In D126907#3617713 , @ChuanqiXu wrote: > In D126907#3615807 , @erichkeane > wrote: > >> All tests pass now, I was able to get the

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3615807 , @erichkeane wrote: > All tests pass now, I was able to get the template-template checks working > correctly, and it passes all the tests I have available. @ChuanqiXu if you > could review/run what tests

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 440712. erichkeane added a comment. Rebased! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 440643. erichkeane added a comment. All tests pass now, I was able to get the template-template checks working correctly, and it passes all the tests I have available. @ChuanqiXu if you could review/run what tests you have, it would be greatly

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane marked an inline comment as done. erichkeane added a comment. In D126907#3600876 , @ChuanqiXu wrote: > Great progress! > > In D126907#3599835 , @erichkeane > wrote: > >> Note that the failure comes

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. Great progress! In D126907#3599835 , @erichkeane wrote: > Note that the failure comes down to: > > template concept C = T::f(); > template class P> struct S1{}; > template struct X{}; > S1 s11; > > and requires the

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Note that the failure comes down to: template concept C = T::f(); template class P> struct S1{}; template struct X{}; S1 s11; and requires the -frelaxed-template-template-args flag: [ekeane1@scsel-clx-24 build]$ ./bin/clang -cc1 -std=c++20 temp.cpp

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 438781. erichkeane added a comment. As promised, got it down to the 1 failure, and added tests for @ChuanqiXu and the std::vector example. That all seems to work, just a problem with the CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp test left. CHANGES

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Made good progress today, but got trapped down the CheckDeducedArgs path for quite a bit longer with partial specializations. I have all of the crashes I know of 'fixed', but have a few tests that still fail for unknown reasons. Because of that, i don't have

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3591195 , @ChuanqiXu wrote: > In D126907#3588708 , @erichkeane > wrote: > >> In D126907#3588417 , @ChuanqiXu >> wrote: >> >>>

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3588708 , @erichkeane wrote: > In D126907#3588417 , @ChuanqiXu > wrote: > >> From what I can see, the crash reason would be the mismatch depth and the >> setting of

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3588417 , @ChuanqiXu wrote: > From what I can see, the crash reason would be the mismatch depth and the > setting of MultiLevelTemplateArgumentList. In > `::CheckConstraintSatisfaction`, the depth of

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. From what I can see, the crash reason would be the mismatch depth and the setting of MultiLevelTemplateArgumentList. In `::CheckConstraintSatisfaction`, the depth of `RawCompletionToken ` is 0, while the depth of corresponding MultiLevelTemplateArgumentList is 2. So

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3585626 , @erichkeane wrote: > See here: https://github.com/llvm/llvm-project/issues/55673 > > This might be the same issue, the crash at least looks the same. Note: Looks to not be the same problem :/

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. See here: https://github.com/llvm/llvm-project/issues/55673 This might be the same issue, the crash at least looks the same. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3584555 , @ChuanqiXu wrote: > I finally found some time to look at the crash. Although I haven't get an > idea, I found it crash at the following one too: > > template > concept Constraint = true; > >

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. I finally found some time to look at the crash. Although I haven't get an idea, I found it crash at the following one too: template concept Constraint = true; template class completion_handler_async_result { public: template static void

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. So from what I can tell, the problem is evaluating `Constraint` on `initiate`, and trying to find the 'right' argument from the `MultiLevelTemplateArgumentList` for `RawCompletionToken`. The problem is that the `RawCompletionToken`, as a template parameter, gets

  1   2   >