[PATCH] D23944: Parsing MS pragma intrinsic

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.

================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:918
@@ +917,3 @@
+  InGroup<IgnoredPragmas>;
+def warn_pragma_intrinsic_builtin_suggest : Warning<
+  "%0 is not a recognized builtin; consider including <intrin.h> to access non-builtin intrinsics">,
----------------
I would combine this with the above and use %select to decide whether to include the "consider including" bit. aka, `"%0 is not a recognized builtin%select{|; consider including <intrin.h> to access non-builtin intrinsics"}1"`.

================
Comment at: lib/Parse/ParsePragma.cpp:2148
@@ +2147,3 @@
+///
+/// Pragma intrisic tells compiler to use a builtin version of function.
+/// Clang does it anyway, so the pragma doesn't really do anything.
----------------
* tells the compiler
* of the function

================
Comment at: lib/Parse/ParsePragma.cpp:2150
@@ +2149,3 @@
+/// Clang does it anyway, so the pragma doesn't really do anything.
+/// Anyway, we emit a warning if the function specified in pragma intrinsic
+/// isn't an intrinsic in clang and suggest to include intrin.h.
----------------
pragma -> #pragma

================
Comment at: lib/Parse/ParsePragma.cpp:2157
@@ +2156,3 @@
+
+  if (Tok.isNot(tok::l_paren)) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen)
----------------
Not for your patch, but I wish we could use the balanced delimiter tracker for parsing pragmas.

================
Comment at: lib/Parse/ParsePragma.cpp:2164
@@ +2163,3 @@
+
+  bool SuggestIntrinH = !PP.isMacroDefined("__INTRIN_H");
+
----------------
Is this safe to rely on? I'm not familiar with how we do our intrinsics, but it's spelled `__INTRIN_H_` in MSVC 2015, but perhaps we only care about intrin.h from Clang?

================
Comment at: lib/Parse/ParsePragma.cpp:2169-2174
@@ +2168,8 @@
+    if (II->getBuiltinID() < 2) {
+      if (SuggestIntrinH) {
+        PP.Diag(Tok.getLocation(), diag::warn_pragma_intrinsic_builtin_suggest)
+            << II;
+      } else {
+        PP.Diag(Tok.getLocation(), diag::warn_pragma_intrinsic_builtin) << II;
+      }
+    }
----------------
Should elide braces here.


https://reviews.llvm.org/D23944