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

Reply via email to