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
setStmts(Stmts);
+ 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`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123952/new/
https://reviews.llvm.org/D123952
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
