[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors
Shafik Yaghmour via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 23 22:04:26 PDT 2023
shafik added a comment.
In D153296#4444612 <https://reviews.llvm.org/D153296#4444612>, @erichkeane wrote:
> So I think I'm pretty confident that the only time we would call `EvaluateDependentExpr` is when we are in an error condition, so I'm convinced the fix 'as is' is incorrect. The check for noteSideEffect records that we HAVE a side effect, then returns if we are OK ignoring them right now.
>
> So since we are in a state where ignoring this error-case is acceptable, I think returning early there is incorrect as well, at least from a 'code correctness' (even if there isn't a reproducer that would matter?). I think we're in a case where we want to continue in order to ensure we go through the entire flow, so I THINK we should treat this as 'we have a value we don't know, so its just not found', and should fall into the check on 5019 (unless of course, there is a 'default' option!).
>
> So I think that we should be checking if `Value` is valid right after the default check, which lets us fall into the 'default' branch and get additional diagnostics/continued evaluation. WDYT @shafik / @yronglin ?
I do see what you are saying but I think that we have similar cases where without a value the next step is impossible to do for example in `EvaluateStmt` the `case Stmt::ReturnStmtClass:` case:
case Stmt::ReturnStmtClass: {
const Expr *RetExpr = cast<ReturnStmt>(S)->getRetValue();
FullExpressionRAII Scope(Info);
if (RetExpr && RetExpr->isValueDependent()) {
EvaluateDependentExpr(RetExpr, Info);
// We know we returned, but we don't know what the value is.
return ESR_Failed;
}
We can't return a value we can't calculate right? and here as well for the `Stmt::DoStmtClass` case
if (DS->getCond()->isValueDependent()) {
EvaluateDependentExpr(DS->getCond(), Info);
// Bailout as we don't know whether to keep going or terminate the loop.
return ESR_Failed;
}
This case feels the same as the two above, we can't calculate the jump for the switch if we can't calculate the value.
CC @rsmith
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153296/new/
https://reviews.llvm.org/D153296
More information about the cfe-commits
mailing list