[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 3 05:42:33 PDT 2023


aaron.ballman added a comment.

In D146764#4313905 <https://reviews.llvm.org/D146764#4313905>, @aeubanks wrote:

> added the warning, not sure if this needs more tests for testing the interaction between `-pedantic`, `-Wmicrosoft`, `-Wmicrosoft-init-from-predefined` or if that's already assumed to work

That's generally assumed to work, really the only question is whether we want `Extension` (only warned in `-pedantic`) or `ExtWarn` (warned by default). I think most of our Microsoft extension warnings are `ExtWarn` and it seems defensible for this to be on by default, but it'd be nice to know just how chatty this is on real world code bases too. I have the sneaking suspicion this code pattern doesn't come up often enough for this to be "too chatty", but if we find it triggers on popular libraries we might want to consider dialing it back to just `Extension`.



================
Comment at: clang/test/Modules/predefined.cpp:6
+// RUN: %clang_cc1 -x c++ -std=c++20 -emit-module-interface a.h -o a.pcm -fms-extensions
+// RUN: %clang_cc1 -std=c++20 a.cpp -fmodule-file=A=a.pcm -fms-extensions -fsyntax-only
+
----------------
aeubanks wrote:
> aaron.ballman wrote:
> > Er, should we have a `-verify` on this as well as `// expected-no-diagnostics`?
> isn't that tested by the Sema test?
Not quite. The Sema test does test a lot of the functionality, but this is testing whether we import the AST node properly and can still use it, so it's testing a different code path for generating the AST nodes used for doing semantic checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764



More information about the cfe-commits mailing list