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

Mon Oct 11 07:25:00 PDT 2021

hans marked an inline comment as done.
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:
> 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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111440