[PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes
Serge Pavlov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 24 02:04:07 PDT 2020
sepavloff added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:2427
+ FPOptions FPFeatures = Cast->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
+ RM = FPFeatures.getRoundingMode();
+ }
----------------
rjmccall wrote:
> sepavloff wrote:
> > rjmccall wrote:
> > > I think the options really need to be passed in or else correctness is somewhat doomed here.
> > >
> > > For example, the call to CompoundAssignSubobjectHandler needs to propagate this down from the operator expression.
> > It is guaranteed by the way AST is built, no?
> >
> > As FP options may be changed only by pragmas and the pragmas can be specified only at file or block level, all sub-expression are evaluated at the same options.
> Yes, but you can't actually reliably recover those settings from E unless you're sure E is one of a few kinds of expression. The concern is that E might end up being some closely-related expression that isn't actually the expression that carries the current settings, and then we'll fall back on using the global defaults. It's much more correct-by-construction to pass the settings down from the caller based on the caller's static knowledge of which expression is under consideration, and I think you'll see that that's pretty straightforward in practice.
This is a peculiarity of `CompoundAssignOperator`, which itself makes conversion, without `CastExpr`. I added `assert` to ensure that other nodes cannot appear here. Also some tests with conversion in `CompoundAssignOperator` were added.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87822/new/
https://reviews.llvm.org/D87822
More information about the cfe-commits
mailing list