[PATCH] D111440: [MS compat] Handle #pragma fenv_access like #pragma STDC FENV_ACCESS (PR50694)

Hans Wennborg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 11 08:00:25 PDT 2021


hans added inline comments.


================
Comment at: clang/lib/Parse/ParsePragma.cpp:280
+    if (Tok.isNot(tok::l_paren)) {
+      PP.Diag(Tok.getLocation(), diag::warn_pragma_ms_fenv_access);
+      return;
----------------
thakis wrote:
> hans wrote:
> > thakis wrote:
> > > Is it important that this is a warning?
> > > 
> > > Independently if we have access to `Parser` here (not sure), there's `Parser::ExpectAndConsume`.
> > > Is it important that this is a warning?
> > 
> > You mean as opposed to an error? Or as opposed to accepting slightly alternative spellings? I don't think we want to accept any other syntax than MSVC. And the custom seems to be to warn and ignore when we can't parse a pragma so I'm following that pattern.
> > 
> > > Independently if we have access to Parser here (not sure), there's Parser::ExpectAndConsume.
> > 
> > Sadly the PragmaHandler classes don't have access to it. It might be possible to plumb it through, but I'm not sure whether it would be worth it.
> > 
> > While looking at it, I did switch to more specific diags for the parens though.
> Yes, as opposed to an error. If cl.exe errs on this, we should probably err too.
They also just warn: https://godbolt.org/z/P3j5Wc78E


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111440/new/

https://reviews.llvm.org/D111440



More information about the cfe-commits mailing list