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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to