yronglin marked 2 inline comments as done. yronglin added a comment. Thanks for your review!
================ Comment at: clang/lib/AST/ExprConstant.cpp:4914 static bool EvaluateDependentExpr(const Expr *E, EvalInfo &Info) { assert(E->isValueDependent()); ---------------- aaron.ballman wrote: > rsmith wrote: > > I don't think the changes to this function are appropriate, because: > > > > 1) The special-casing of `RecoveryExpr` doesn't seem like it can be > > correct. There's no guarantee that we get a `RecoveryExpr` any time we > > encounter an expression that contains errors; error-dependence can be > > propagated from other places, such as types. > > 2) For other error-dependent expressions, we also can't necessarily compute > > a value. > > 3) It's not the responsibility of this function to deal with the situation > > where a value is needed and can't be produced -- the responsibility to > > handle that lies with the caller of this function instead. Eg, look at the > > handling of `ReturnStmt` or `DoStmt`. > > > > So I think we should undo all the changes in this function, and only fix > > `SwitchStmt` to properly handle a value-dependent condition. > Thank you for the explanation -- this makes more sense to me now. I agree > with the suggestion to just change `EvaluateSwitch()`, sorry for the false > start! Thanks! I have undo this change. 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