rjmccall added a comment.
Just a bunch of minor suggestions. LGTM if you get all the tests worked out
and it actually works the way you want on the tests.
================
Comment at: clang/include/clang/AST/Expr.h:2280
}
+ FPOptionsOverride getFPFeatures(const LangOptions &LO) const {
+ if (UnaryOperatorBits.HasFPFeatures)
----------------
rjmccall wrote:
> I would call this one getFPOptionOverrides or getOverriddenFPOptions. If
> you're going to have a getFPFeatures, it really ought to be the other one —
> although since we're changing pretty much all of the call sites, we might as
> well rename it to getFPOptions (or getFPOptionsInEffect, that seems fine,
> too).
I don't think there's any reason for this to take a LangOptions, since the set
of overrides should be independent of the global settings.
================
Comment at: clang/include/clang/Basic/LangOptions.h:512
+ FPOptions result( (Base.getAsOpaqueInt() & ~OverrideMask)
+ | (OverrideMask & Options.getAsOpaqueInt()));
+ return result;
----------------
clang-format's suggestion is good here
================
Comment at: clang/include/clang/Basic/LangOptions.h:542
+ LLVM_DUMP_METHOD void dump();
+};
----------------
Same here: clang-format's just asking you to right-justify the escapes, and
it's right, that's the style we usually use with multi-line macros.
================
Comment at: clang/include/clang/Sema/Sema.h:569
+ }
// RAII object to push / pop sentinel slots for all MS #pragma stacks.
----------------
Spurious spaces (as pointed out by clang-format)
================
Comment at: clang/include/clang/Serialization/ASTWriter.h:509
void WriteDeclContextVisibleUpdate(const DeclContext *DC);
- void WriteFPPragmaOptions(const FPOptions &Opts);
+ void WriteFPPragmaOptions(const FPOptionsOverride &Opts);
void WriteOpenCLExtensions(Sema &SemaRef);
----------------
I think it's telling you that you're missing a forward-declaration of this type
in this file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81869/new/
https://reviews.llvm.org/D81869
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
