[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

shafik added a comment. Looks like this bug is caused by this commit: https://github.com/llvm/llvm-project/issues/63782#issuecomment-1633312909 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - I took your example and tried to reduce it further https://godbolt.org/z/jEx9vdj7K It's kind of a difficult situation - both gcc and msvc accept it, yet /* very very cautiously */ it might happen that the code is actually invalid ... (i'd nee

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. There is probably more reduction work to be done, but I hit the end of my day here: https://godbolt.org/z/Tzfx31asK Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 __

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4343477 , @alexander-shaposhnikov wrote: > @erichkeane - I'll have stable internet ~soon and will try to look into the > reported issue (but help would be greatly appreciated). > To the best of my knowledge there a

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - I'll have stable internet ~soon and will try to look into the reported issue (but help would be greatly appreciated). To the best of my knowledge there are other problems with libstdc++'s ranges (even without this diff), but yeah, this regres

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4339561 , @awson wrote: > Now, this > > #include > #include > > auto drop1(const std::vector& s){ > return s | std::views::drop(1); > } > > when compiled against gcc's-13.1 libstdc++ spits: > > b

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

awson added a comment. Now, this #include #include auto drop1(const std::vector& s){ return s | std::views::drop(1); } when compiled against gcc's-13.1 libstdc++ spits: boro.cpp:5:11: error: invalid operands to binary expression ('const std::vector' and '_Partial<_Drop,

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. I see this is closed... but I accept if you wish to re-commit this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___ cfe-commits

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( alexander-shaposhnikov wrote: > alexander-shaposhnikov wrote: > > alexander-shaposhnikov wrote: > > > alexander-shaposhniko

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( alexander-shaposhnikov wrote: > alexander-shaposhnikov wrote: > > alexander-shaposhnikov wrote: > > > HandlePar

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( alexander-shaposhnikov wrote: > alexander-shaposhnikov wrote: > > HandlePartialClassTemplateSpec is from Erich'

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @davidtgoldblatt - thanks for the report. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___ cfe-commits mailing list c

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/test/SemaTemplate/concepts-out-of-line-def.cpp:348 + +namespace MultilevelTemplateWithPartialSpecialization { +template (new tests) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( alexander-shaposhnikov wrote: > HandlePartialClassTemplateSpec is from Erich's diff > (https://reviews.llvm.or

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 519711. alexander-shaposhnikov added a comment. Remove dead code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/include/clang/AST/Decl

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 519708. alexander-shaposhnikov added a comment. Add more tests, Fix HandlePartialClassTemplateSpec. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Fil

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. Reverted in https://github.com/llvm/llvm-project/commit/3b9ed6e5323176550925f3b0a2c50ced1b61438d, it'll take time to investigate this case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://r

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

davidtgoldblatt added a comment. This version of the commit also introduces some breakages; as before I'm not sure if it's the code or the diff that's incorrect. Repro: enum class Enum { E1 }; template inline constexpr bool some_concept = true; template struct S { template

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. P.S. Landed. If it survives in the trunk this time - I'll send a follow-up diff with the release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - thanks, then I'm going to give it another try. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___ cfe-com

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. Missing context in the diff, but the changes since commit are all sensible to me. If this fixes everything we know about, I'm OK with it. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:344 Innermost->as

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

sberg added a comment. In D146178#4314999 , @sberg wrote: > Since this commit... ah, already taken care of with https://github.com/llvm/llvm-project/commit/3e850a6eea5277082a0b7b701754c86530d25c40 "Revert '[Clang][Sema] Fix comparison of constraint exp

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

sberg added a comment. Since this commit (plus its follow-up https://github.com/llvm/llvm-project/commit/ce861ec782ae3f41807b61e855512aaccf3c2149 "[Clang][Sema] Add a temporary workaround in SemaConcept.cpp", to avoid `clang/lib/AST/ExprConstant.cpp:15332: bool clang::Expr::EvaluateAsConstantE

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( HandlePartialClassTemplateSpec is from Erich's diff (https://reviews.llvm.org/D147722) Comm

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 518990. alexander-shaposhnikov added a comment. Simplify code a little bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/include/clan

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 518966. alexander-shaposhnikov added a comment. 1. If innermost != nullptr (in getTemplateInstantiationArgs) and NS is a ClassTemplatePartialSpecializationDecl we were incorrectly adding the inner level of template args twice (once as an array

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. upd. In the reduced example above MLTAL is incorrect (lldb) p MLTAL.dump() NumRetainedOuterLevels: 1 1: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. reduced test case: template concept Concept = false; struct Foo { template struct result {}; template requires(Concept<_Tp>) struct result<_Tp>; }; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4313058 , @alexander-shaposhnikov wrote: > @erichkeane - feel free to take over this patch. If I get time, I will! Else it'll be here when you get back :) Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - feel free to take over this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___ cfe-commits mailin

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. I reverted due to the workaround (which I'd objected to before, I don't think it is acceptable) and the breakage David mentioned. I'll make sure to review the commit message when you put it back up for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4312473 , @davidtgoldblatt wrote: > This breaks ~the world in the versions of libstdc++ I can easily check -- see > e.g. https://gcc.godbolt.org/z/ETeGzc3ve (crashes at this commit, changes to > an error at ce861e

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

davidtgoldblatt added a comment. This breaks ~the world in the versions of libstdc++ I can easily check -- see e.g. https://gcc.godbolt.org/z/ETeGzc3ve (crashes at this commit, changes to an error at ce861ec782ae3f41807b61e855512aaccf3c2149

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - thanks! I'll send a diff for the release notes ~soon (~this week). (P.S. just in case - I'll be out of office for ~2 weeks) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://r

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. Hey! I wish you'd given me a chance to review this fix version before committing! I don't have concerns now looking at it, however please add a release note, since this is fixing a whole-bunch of regressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe3b1083e00e6: [Clang][Sema] Fix comparison of constraint expressio

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 517513. alexander-shaposhnikov added a comment. Add more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/include/clang/AST/DeclTem

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. /* will update the diff ~soon */ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___ cfe-commits mailing list cfe-commit

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov planned changes to this revision. alexander-shaposhnikov added a comment. /* working on it */ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. reduced test case (that is currently blocking this diff) : namespace outer::internal { template concept myconcept = true; } namespace outer { template class Foo; template struct Bar { template friend class Foo; };

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. Yeah, things appear to work for the current version of this diff (including GH62110). However, while doing some internal testing I've discovered one suspicious issue (sigh, sigh), but haven't been able to create a standalone repro yet. Need more time for d

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. Sorry I haven't been able to help yet, I'm STILL dealing with IT issues with the server I work on. I appreciate your patience! I have no comments, this all looks fine to me. Does this fix 62110 as well? Is there anythign to hold this up from landing? Repository:

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; alexander-shaposhnikov wrote: > alexander-shaposhnikov wrote: > > erichkeane

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:781 + /*ForConstraintInstantiation=*/true, /*SkipForSpecialization*/ false); + Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false); + std::optional ThisScope; a

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:781 + /*ForConstraintInstantiation=*/true, /*SkipForSpecialization*/ false); + Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false); + std::optional ThisScope; r

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 514745. alexander-shaposhnikov added a comment. New version (address some comments) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/incl

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; alexander-shaposhnikov wrote: > erichkeane wrote: > > alexander-shaposhnikov

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; erichkeane wrote: > alexander-shaposhnikov wrote: > > erichkeane wrote: > > >

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; alexander-shaposhnikov wrote: > erichkeane wrote: > > alexander-shaposhnikov wrote: > > >

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:227 +(TSTy = Ty->getAs())) + Result.addOuterTemplateArguments(const_cast(FTD), + TSTy->template_arguments(), er

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:227 +(TSTy = Ty->getAs())) + Result.addOuterTemplateArguments(const_cast(FTD), + TSTy->template_arguments(), So I'd come up

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. https://github.com/llvm/llvm-project/issues/62110 is still a blocker for this diff though (haven't investigated it yet) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 514095. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/include/clang/Sema/Template.h clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/Se

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov planned changes to this revision. alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstant

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > erichkeane wrote:

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > alexander-shaposh

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > alexa

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > alexander-shaposh

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; alexander-shaposhnikov wrote: > erich

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. https://github.com/llvm/llvm-project/issues/62110 (in addition to the self-friendship case) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. Requesting changes so we don't accidentally commit this. There is some confusion elsewhere on how ready this is for commit. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; erichkeane wrote: > alexander-shaposhnikov wrote: > > erichkeane wrote: > > >

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > alexa

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; alexander-shaposhnikov wrote: > erich

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; alexander-shaposhnikov wrote: > erichkeane wrote: > > So this bit is concerning to me...

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:773 + // ConstrExpr for the inner template will properly adjust the depths. + if (isa(ND) && isa(OtherND)) +ForConstraintInstantiation = true; erichkeane wrote: > Hmm..

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; erichkeane wrote: > So this bit is concerning to me... we have been catching

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:263 + if (SubstitutedAtomicExpr.get()->isValueDependent()) +return SubstitutedAtomicExpr; So this bit is concerning to me... we have been catching a ton of bugs in the MLTAL stuf

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 511854. alexander-shaposhnikov added a comment. This version is partially based on https://reviews.llvm.org/D147722. The case with self-friendship is problematic (had to add a workaround, any suggestions would be greatly appreciated). Added mor

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. I've debugged a bit what's going on in https://godbolt.org/z/7h3sPe85h we pass ForConstaintInstantiation=true and this causes us for the in-class FunctionTemplateDecl pick up the outer layer of template args (i.e. MLTAL will contain NumRetainedOuterLevels:

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

ilya-biryukov added a comment. This breaks the following innocent looking out-of-line definition with two levels of constrained template parameters: template concept Result = true; template class CoFuture final { template explicit CoFuture(); }; template template CoF

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG60bee9ff5445: [Clang][Sema] Fix comparison of constraint expressions (authored by alexander-shaposhnikov). Repository: rG LLVM Github Monorepo CH

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 510662. alexander-shaposhnikov added a comment. Add test & minor optimization CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaOve

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 510637. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTe

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Does my horrible lambda example work now? If so, that seems like a useful testcase. Comment at: clang/lib/Sema/SemaConcept.cpp:781 + /*ForConstraintInstantiation=*/true

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 510634. alexander-shaposhnikov added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/lib/Sema/SemaConcept.cp

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:775-776 +static bool IsInsideImplicitFullTemplateSpecialization(const DeclContext *DC) { + auto *CTSDecl = dyn_cast_or_null( + DC->getOuterLexicalRecordContext()); + return CTSDecl && !isa(CTSDecl) &&

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith added a comment. Can you also change www/cxx_status.html to list P2103R0 as supported in the most recent version of Clang rather than in some previous version? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.o

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:775-776 +static bool IsInsideImplicitFullTemplateSpecialization(const DeclContext *DC) { + auto *CTSDecl = dyn_cast_or_null( + DC->getOuterLexicalRecordContext()); + return CTSDecl && !isa(CTSDecl) &&

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @rsmith - thanks a lot for the review, is there anything you'd like me to do on this diff or we are good to go ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 __

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 509910. alexander-shaposhnikov added a comment. Rebased + rerun all the tests + internal testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 509546. alexander-shaposhnikov added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 Files: clang/lib/Sema/SemaConcept.cp

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith added a comment. This all looks good to me. Sorry for the delay. Comment at: clang/lib/Sema/SemaConcept.cpp:728 bool SkipForSpecialization = false) { MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( ND

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4214972 , @alexander-shaposhnikov wrote: > Add more tests. > P.S. we already have tests with self-friends (in concepts.cpp), the test from > Richard's comment is also included (struct S12) (in a slightly simplified

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 507560. alexander-shaposhnikov added a comment. Add more tests. P.S. we already have tests with self-friends (in concepts.cpp), the test from Richard's comment is also included (struct S12) (in a slightly simplified form) Repository: rG LLV

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov planned changes to this revision. alexander-shaposhnikov added a comment. will add more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1676 +Inst->setLexicalDeclContext(Owner); +RecordInst->setLexicalDeclContext(Owner); + this bit is important Repository: rG LLVM Github Monorepo

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4213943 , @alexander-shaposhnikov wrote: > @erichkeane - thanks for the comments, the changes in > SemaTemplateInstantiateDecl.cpp are necessary, in particular, they enable us > to handle the case > > template

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - thanks for the comments, the changes in SemaTemplateInstantiateDecl.cpp are necessary, in particular, they enable us to handle the case template concept Constraint = true; template struct Iterator { template friend clas

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. Hmm... there is a lot going on here that I'm not sure is necessary? I was able to change the `AreConstraintExpressionsEqual` 'if' body to just: +MultiLevelTemplateArgumentList OldMLTAL = +getTemplateArgumentsForComparison(*this, Old); +MultiLevel

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov updated this revision to Diff 507432. alexander-shaposhnikov edited the summary of this revision. alexander-shaposhnikov added a comment. New approach to constraints' comparison Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:728 bool SkipForSpecialization = false) { MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( ND, /*Final=*/false, /*Innermost=*/nullptr

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4213235 , @alexander-shaposhnikov wrote: > @erichkeane - yes, I'm working on it, I hope to have a new version ~soon > (within a few days). Ok, thanks for the update then! I'll do just enough looking into it to be

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @erichkeane - yes, I'm working on it, I hope to have a new version ~soon (within a few days). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 _

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. @alexander-shaposhnikov : Are you working on @rsmith 's suggestion? I finally have time to look into it, but dont want to repeat effort if you're already 1/2 way into it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

erichkeane added a comment. In D146178#4200821 , @rsmith wrote: > The approach of attempting to adjust the depth here is not correct, and we > should rip this whole mechanism out and reimplement it. Consider this > closely related testcase: > > templ

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

rsmith added a comment. The approach of attempting to adjust the depth here is not correct, and we should rip this whole mechanism out and reimplement it. Consider this closely related testcase: template concept Concept = true; template struct A { template V> void C(V&& t)

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

alexander-shaposhnikov added a comment. @shafik - it's a bit early to review this patch, this was the first attempt to fix the issue related to the current behavior of clang::Sema::TemplateParameterListsAreEqual that causes Clang to mishandle out-of-line definitions involving constraints (and

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

shafik added a comment. I also don't get what is going on here, either a more detailed summary or more inline comments should help. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:1303 +OldTemplate->getTemplateParameters(), false, TPL_TemplateMatch, +SourceLocation(), false /* PartialOrdering */); bool SameReturnType = Context.hasSameType(Old->getDeclaredReturnTy

  1   2   >