yronglin added a comment. In D153296#4468373 <https://reviews.llvm.org/D153296#4468373>, @aaron.ballman wrote:
> In D153296#4459769 <https://reviews.llvm.org/D153296#4459769>, @yronglin > wrote: > >> Please help me, I have no better idea on this issue, do you have any better >> ideas? @erichkeane @shafik > > I think what's being suggested is to change `EvaluateDependentExpr()` > somewhat along these lines: > > static bool EvaluateDependentExpr(const Expr *E, EvalInfo &Info) { > assert(E->isValueDependent()); > > // Note that we have a side effect that matters for constant evaluation. > bool SideEffects = Info.noteSideEffect(); > // If the reason we're here is because of a recovery expression, we don't > // want to continue to evaluate further as we will never know what the > actual > // value is. > if (isa<RecoveryExpr>(E)) > return false; > > // Otherwise, return whether we want to continue after noting the side > // effects, which should only happen if the expression has errors but > isn't > // a recovery expression on its own. > assert(E->containsErrors() && "valid value-dependent expression should > never " > "reach invalid code path."); > return SideEffects; > } > > This way, code paths that get down to a `RecoveryExpr` will not continue to > evaluate further (as there's really no point -- there's no way to get a > reasonable value from from the recovery expression anyway), but the fix isn't > specific to just switch statements. After making these changes, you should > look for places where `EvaluateDependentExpr()` is being called to try to > come up with a test case where that expression is a recovery expression so > that we can fill out test coverage beyond just the situation with `switch` > from the original report. Does that make sense? > > (Marking as requesting changes so it's clear this review isn't yet accepted.) Thanks a lot! @aaron.ballman , I try to address comments and add more test, this case (https://godbolt.org/z/ExPoEKcrf) looks strange, why the do-statement missing in the printed AST? 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
