hokein added a comment.
Thanks for the fix. Sorry for being late (I was looking through the emails and
found this patch).
================
Comment at: clang/lib/AST/ExprConstant.cpp:4921
+ // value is.
+ if (isa<RecoveryExpr>(E))
+ return false;
----------------
The constant evaluator is not aware of the "error" concept, it is only aware of
value-dependent -- the general idea behind is that we treat the
dependent-on-error and dependent-on-template-parameters cases the same, they
are potentially constant (if we see an expression contains errors, it could be
constant depending on how the error is resolved), this will give us nice
recovery and avoid bogus following diagnostics.
So, a `RecoveryExpr` should not result in failure when checking for a potential
constant expression.
I think the right fix is to remove the conditional check `if
(!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and return
`ESR_Failed` unconditionally (we don't know its value, any switch-case anwser
will be wrong in some cases). We already do this for return-statment,
do-statement etc.
================
Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:91
+
+static_assert(test13(), "should not crash"); // expected-error {{static
assertion expression is not an integral constant expression}}
+
----------------
nit: we can simplify it with the `TEST_EVALUATE` macro:
```
TEST_EVALUATE(SwitchErrorCond, switch(undef) { case 0: return 7; default:
break;})
```
================
Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:93
+
+constexpr int test14() {
+ int sum = 0;
----------------
Is this a new crash (and the tests below)?
They don't look like new crashes, I think the current constant evaluator should
be able to handle them well. IIUC the only crash we have is the case where we
have a error-dependent condition in `switch`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153296/new/
https://reviews.llvm.org/D153296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
