erichkeane added a comment.
2 small things, @rjmccall and @sepavloff , anything else?
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:870
+def err_pragma_fenv_requires_precise : Error<
+ "'#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled">;
def warn_cxx_ms_struct :
----------------
The last 4 can be done via selects as well! Save a couple more spaces before
we have to up the diagnostic id size :)
================
Comment at: clang/include/clang/Sema/Sema.h:558
+ // This stacks the current state of Sema.CurFPFeatures
+ PragmaStack<unsigned> FpPragmaStack;
----------------
erichkeane wrote:
> This comment is really oddly phrased and uses the 'stack'-noun as a verb?
> Something like: (please feel free to wordsmith): "This stack tracks the
> current state of Sema.CurFPFeatures."?
Just needs a period at the end.
================
Comment at: clang/lib/Parse/ParsePragma.cpp:2534
+ PP.Lex(Tok);
+ if (Tok.isNot(tok::l_paren)) {
+ PP.Diag(FloatControlLoc, diag::err_expected) << tok::l_paren;
----------------
mibintc wrote:
> erichkeane wrote:
> > Replace this with BalancedDelimiterTracker instead, it gives consistent
> > errors and are a bit easier to use. Additionally, I think it does some
> > fixups that allow us to recover better.
> >
> > I'd also suggest some refactoring with the PragmaFloatControlKind if/elses
> > below. Perhaps handle the closing paren at the end, and do a switch-case
> > for that handling.
> BalancedDelimiterTracker doesn't work here because there's no access to the
> Parser object. Rewriting it would be an extensive change and I'm doubtful
> about making this change. PragmaHandler is defined in Lex. I think there are
> 60 pragma's that use the PragmaHandler.
Thats unfortunate :/ That type does some nice fixup work.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72841/new/
https://reviews.llvm.org/D72841
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
