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 project that I'd like to make sure doesn't
get stuck in a rebase-loop again.
================
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<Expr*>(AtomicExpr));
----------------
Can you explain this more? How does this work, and why don't we do that
directly instead?
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2042
+ !SemaRef.CheckConstraintExpression(TransConstraint.get())) {
+ assert(Trap.hasErrorOccurred() && "CheckConstraintExpression failed, but
"
+ "did not produce a SFINAE error");
----------------
This branch ends up being empty if asserts are off. Also, it results in
CheckConstraintExpression happening 2x, which ends up being more expensive
after https://reviews.llvm.org/D126907
================
Comment at: clang/test/SemaTemplate/concepts-PR54629.cpp:10
+int main() {
+ A<int> a;
+}
----------------
Simply 'doesn't crash' isn't quite enough for a test here, I would like to see
some level of confirmation which of the versions of "A" get selected here. So
perhaps `A<double>{}.some_func();` call that wouldn't be valid/etc. And
perhaps a situation where both instances have a constraint and and we diagnose
why it doesn't fit?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127487/new/
https://reviews.llvm.org/D127487
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
