[PATCH] D31166: Encapsulate FPOptions and use it consistently

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/AST/ExprCXX.h:58
+  // Only meaningful for floating point types. For other types this value can be
   // set to false.
+  FPOptions FPFeatures;
----------------
Setting a class to false sounds a bit strange. May want to reword.


================
Comment at: include/clang/Basic/LangOptions.h:180
 
-  FPOptions(const LangOptions &LangOpts) :
-    fp_contract(LangOpts.DefaultFPContract) {}
+  explicit FPOptions(uint64_t I) : fp_contract(I) {}
+
----------------
Why a `uint64_t` rather than `unsigned`? Same for the accessor.


================
Comment at: include/clang/Basic/LangOptions.h:182
+
+  FPOptions(const LangOptions &LangOpts)
+      : fp_contract(LangOpts.DefaultFPContract) {}
----------------
Might as well make this one explicit as well.


================
Comment at: lib/CodeGen/CGExprScalar.cpp:1712
   BinOp.Opcode = IsInc ? BO_Add : BO_Sub;
-  BinOp.FPContractable = false;
+  // FIXME: once UnaryOperator carries FPFeatures, copy it here.
   BinOp.E = E;
----------------
Why not make UnaryOperator carry this information now, since it's needed?


https://reviews.llvm.org/D31166