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

Melanie Blower via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 13 12:26:21 PDT 2020


mibintc marked 3 inline comments as done.
mibintc added a comment.

Adding an inline reply for John. rebased the patch, also re-applied clang-format.  What do you think?



================
Comment at: clang/lib/Analysis/BodyFarm.cpp:120
+                                VK_RValue, OK_Ordinary, SourceLocation(),
+                                FPOptions::defaultWithoutTrailingStorage(C));
 }
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384





More information about the cfe-commits mailing list