rsmith added inline comments.
================ Comment at: clang/include/clang/Sema/Sema.h:10298 + bool AllowFold = true, + bool StoreResult = true); ExprResult VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result, ---------------- Tyker wrote: > rsmith wrote: > > Do you need this new flag? No callers are passing it. > the idea behind it is that not all integral constant expression benefit from > storing the result. i have not analyzed which benefit from it and which > don't. adding a FIXME would have probably be more appropriate. It's useful to have a consistent AST representation, so I'd be inclined to at least always create a `ConstantExpr` node. Also, once we start allowing `consteval` evaluations that have side effects (for example, reflection-related actions that modify the AST), I think we'll need to always store the result to ensure we never evaluate a constant expression twice (and trigger its side-effects to occur twice). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63376/new/ https://reviews.llvm.org/D63376 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits