[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Analysis/BodyFarm.cpp:120
+                                VK_RValue, OK_Ordinary, SourceLocation(),
+                                FPOptions::defaultWithoutTrailingStorage(C));
 }
----------------
mibintc wrote:
> rjmccall wrote:
> > These construction sites don't seem like appropriate uses of `defaultWithoutTrailingStorage`, which is an implementation detail of the AST nodes.  The right behavior here is for all of these places to use the default `FPOptions` from the language options, then let the AST figure out the right way to store that.  That happens to have the same effect currently as `defaultWithoutTrailingStorage`, but the latter should be able to change, while the right behavior for these places will remain the same.
> OK i changed these in BodyFarm to use the default constructor.  In Sema I changed them from defaultWithoutTrailingStorage to use Sema.FPFeatures which is the current setting combining pragma values + command line options. 
That's right for Sema.  For all the other generated uses, I think you should use `FPOptions(C.getLangOpts())` instead of the default constructor.  Most of these don't really care about FPOptions — they're just creating an assignment or some other operation which is not FP-sensitive — but for the ones that do, we should follow the default behavior in the language options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76384/new/

https://reviews.llvm.org/D76384