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 rather collapse this into SOMETHING like:
>
> ``` assert((TransConstraint.isInvalid() ||
> (SemaRef.CheckConstraintExpr(TransConstraint.get() ||
> Trap.hasErrorOccurred()) && "...");```
>
> I think thats right?
`CheckConstraintExpression` has to always run, even when assertions are
disabled. It does checks for C++ semantics, e.g. checks that the type is bool.
The assertion is only there to check that whenever the function fails, `Trap`
captures the actual diagnostic so the code below that relies on
`hasErrorOccurred` ends up in a valid state.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2048
+ // Use version of CheckConstraintSatisfaction that does no substitutions.
+ if (!TransConstraint.isInvalid() &&
+ !TransConstraint.get()->isInstantiationDependent() &&
----------------
erichkeane wrote:
> Same sorta thing here, if our check is only used for an assert, we should
> call it in the assert.
See the comment above, the call to `CheckConstraintSatisfaction` is also to
implement semantic C++ checks, not solely for assertion.
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
