[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG342e64979afe: [Sema] Fix assertion failure when instantiating requires expression (authored by ilya-biryukov). Changed prior to commit: https://re

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Ah, i see! Thanks for the explanation! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127487/new/ https://reviews.llvm.org/D127487

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2039 TransConstraint = TransformExpr(Req->getConstraintExpr()); +if (!TransConstraint.isInvalid()) { + bool CheckSucceeded = erichkeane wrote: > I think I'd

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2039 TransConstraint = TransformExpr(Req->getConstraintExpr()); +if (!TransConstraint.isInvalid()) { + bool CheckSucceeded = I think I'd rather collapse this in

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. @erichkeane could you take another look at this? Comment at: clang/test/SemaTemplate/concepts-PR54629.cpp:10 +int main() { + A a; +} ilya-biryukov wrote: > erichkeane wrote: > > Sim

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov updated this revision to Diff 438977. ilya-biryukov added a comment. - Update test, check error messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127487/new/ https://reviews.llvm.org/D127487 Files: clang/lib/Sema/SemaConcept.c

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2042 +!SemaRef.CheckConstraintExpression(TransConstraint.get())) { + assert(Trap.hasErrorOccurred() && "CheckConstraintExpression failed, but " +

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2042 +!SemaRef.CheckConstraintExpression(TransConstraint.get())) { + assert(Trap.hasErrorOccurred() && "CheckConstraintExpression failed, but " +

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:352 + [this](const Expr *AtomicExpr) -> ExprResult { +// We only do this to immitate lvalue-to-rvalue conversion. +return PerformContextuallyConvertToBool(const_cast(AtomicExpr));

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane added a comment. In D127487#3573667 , @ilya-biryukov wrote: > I've checked that the change works fine on top of D126907 > . The bug is still there after D126907 > and gets

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added a comment. I've checked that the change works fine on top of D126907 . The bug is still there after D126907 and gets fixed by this. Also, the merge conflict is actually minimal, no code changes intersect.

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

royjacobson added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:352 + [this](const Expr *AtomicExpr) -> ExprResult { +// We only do this to immitate lvalue-to-rvalue conversion. +return PerformContextuallyConvertToBool(const_cast(AtomicExpr)); -

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:352 + [this](const Expr *AtomicExpr) -> ExprResult { +// We only do this to immitate lvalue-to-rvalue conversion. +return PerformContextuallyConvertToBool(const_cast(AtomicExpr)); --

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added a comment. In D127487#3573180 , @erichkeane wrote: > I'm not quite understanding this yet, so I'll have to take another look early > next week. However, I AM intending to get https://reviews.llvm.org/D126907 > committed in the next

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov updated this revision to Diff 435908. ilya-biryukov added a comment. - Do not introduce empty branches on asserts - Test whether specialization or primary template gets picked Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127487/new/

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

erichkeane added a comment. I'm not quite understanding this yet, so I'll have to take another look early next week. However, I AM intending to get https://reviews.llvm.org/D126907 committed in the next week or so. Could you perhaps see how it interacts with that? Its a sizable, multi-month

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added a comment. Fixed issue is: 54629 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127487/new/ https://reviews.llvm.org/D127487 ___ cfe

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov added a subscriber: jyknight. ilya-biryukov added a comment. @saar.raz @erichkeane please let me know if you are the right people to review this. The change might need a little more refactoring, e.g. placement new should probably move to a helper in Sema to align with what clang us

[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

ilya-biryukov created this revision. ilya-biryukov added reviewers: erichkeane, saar.raz. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang. Fixes #54629. The crash is is caused by the double template instantiation. See the added test. Here