[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Shafik Yaghmour via Phabricator via cfe-commits]

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