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

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 09:53:29 PDT 2023


erichkeane added a comment.

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 ?


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