[PATCH] D123952: [FPEnv] Allow CompoundStmt to keep FP options

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 27 12:17:17 PDT 2022

aaron.ballman added inline comments.

Comment at: clang/include/clang/Sema/ScopeInfo.h:77-78
-  CompoundScopeInfo(bool IsStmtExpr) : IsStmtExpr(IsStmtExpr) {}
+  /// FP options at the beginning of the compound statement, prior to
+  /// any pragma.
+  FPOptions FPFeatures;
sepavloff wrote:
> aaron.ballman wrote:
> > So these are the initial FPOptions inherited by the scope from its surrounding context? And it's never updated by a pragma?
> Yes, this FPOption is used to calculate the effect of pragmas in the compound statement as a difference with FPOptions stored in Sema, so that CompoundStmt keeps only FP features that are changed in it.
It might make sense to rename this to `OriginalFPFeatures` or `InitialFPFeatures` or something like that, to make it more clear that this field is not updated as we process the rest of the compound statement. WDYT?

Comment at: clang/lib/AST/Stmt.cpp:370-371
+  if (hasStoredFPFeatures())
+    setStoredFPFeatures(FPFeatures);
There's a part of me that wonders if it's a slightly more clear design to have `setStoredFPFeatures()` set `CompoundStmtBits.HasFPFeatures` to true as needed rather than requiring the caller to do this two-step initialization process. WDYT?

Comment at: clang/lib/AST/StmtPrinter.cpp:216
+    LangOptions::RoundingMode RM = FPO.getConstRoundingModeOverride();
+    if (RM != llvm::RoundingMode::Dynamic) {
+      Indent() << "#pragma STDC FENV_ROUND ";
Why is this not handled as well and print out `FE_DYNAMIC`?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list