aaron.ballman added a comment.
Thanks for working on this! One thing that's not clear to me is what bugs this
is solving -- the test coverage only shows a change to textual AST dumping
behavior. Is this otherwise expected to be an NFC change, or should there be
some codegen tests that show a difference in behavior?
================
Comment at: clang/include/clang/AST/Stmt.h:134
+ /// floating-point features.
+ unsigned HasFPFeatures : 1;
+
----------------
I don't think this is a bad approach, but it does further reduce the number of
statements we support in a compound statement.
There's a part of me that wonders if the approach is to introduce a new AST
node for a stateful compound statement; I suspect if we dig around, we'll find
other pragmas that want to behave similar to floating-point feature pragmas
(the idea of a pragma scoped to a block is not really new). Having something
more general means we're less likely to keep stealing bits here.
================
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;
----------------
So these are the initial FPOptions inherited by the scope from its surrounding
context? And it's never updated by a pragma?
================
Comment at: clang/lib/AST/TextNodeDumper.cpp:2372-2376
+void TextNodeDumper::VisitCompoundStmt(const CompoundStmt *S) {
+ VisitStmt(S);
+ if (S->hasStoredFPFeatures())
+ printFPOptions(S->getStoredFPFeatures());
+}
----------------
Should we have a similar change for the JSON node dumper?
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
