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

[Nico Weber via Phabricator via cfe-commits]

thakis 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;
----------------
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.


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

https://reviews.llvm.org/D111440