<div dir="ltr">Thanks! Some bikesheddy comments, ignore as you see fit:<div><br></div><div>- I think it's good to keep Chromium's bug tracker out of LLVM as far as possible (most LLVM devs don't need to care about Chromium, and since Chromium's bug tracker refers to LLVM's bug tracker frequently since chromium depends on llvm, adding links the other way round adds a dependency cycle of sorts), so maybe describe the motivation in your own words instead of linking ("People might want to receive warnings about pragmas but not about intrinsics we don't yet implement")</div><div><br></div><div>- Should this be part of -Wunknown-pragma instead of -Wignored-pragma? (Or maybe even in -Wmicrosoft somewhere?) That seems to fit a tiny bit better imho (but as I said, feel free to disagree and ignore)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 20, 2016 at 2:38 PM, Albert Gutowski <span dir="ltr"><<a href="mailto:agutowski@google.com" target="_blank">agutowski@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">agutowski created this revision.<br>
agutowski added reviewers: thakis, hans.<br>
agutowski added a subscriber: cfe-commits.<br>
<br>
<a href="https://bugs.chromium.org/p/chromium/issues/detail?id=644841#c9" rel="noreferrer" target="_blank">https://bugs.chromium.org/p/<wbr>chromium/issues/detail?id=<wbr>644841#c9</a><br>
<br>
<a href="https://reviews.llvm.org/D24775" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24775</a><br>
<br>
Files:<br>
  include/clang/Basic/<wbr>DiagnosticGroups.td<br>
  include/clang/Basic/<wbr>DiagnosticParseKinds.td<br>
  test/Preprocessor/pragma_<wbr>microsoft.c<br>
<br>
Index: include/clang/Basic/<wbr>DiagnosticParseKinds.td<br>
==============================<wbr>==============================<wbr>=======<br>
--- include/clang/Basic/<wbr>DiagnosticParseKinds.td<br>
+++ include/clang/Basic/<wbr>DiagnosticParseKinds.td<br>
@@ -914,7 +914,7 @@<br>
 // - #pragma intrinsic<br>
 def warn_pragma_intrinsic_builtin : Warning<<br>
   "%0 is not a recognized builtin%select{|; consider including <intrin.h> to access non-builtin intrinsics}1">,<br>
-  InGroup<IgnoredPragmas>;<br>
+  InGroup<<wbr>IgnoredPragmaIntrinsic>;<br>
 // - #pragma unused<br>
 def warn_pragma_unused_expected_<wbr>var : Warning<<br>
   "expected '#pragma unused' argument to be a variable name">,<br>
Index: include/clang/Basic/<wbr>DiagnosticGroups.td<br>
==============================<wbr>==============================<wbr>=======<br>
--- include/clang/Basic/<wbr>DiagnosticGroups.td<br>
+++ include/clang/Basic/<wbr>DiagnosticGroups.td<br>
@@ -439,8 +439,9 @@<br>
 def UninitializedStaticSelfInit : DiagGroup<"static-self-init">;<br>
 def Uninitialized  : DiagGroup<"uninitialized", [UninitializedSometimes,<br>
                                                  UninitializedStaticSelfInit]>;<br>
+def IgnoredPragmaIntrinsic : DiagGroup<"ignored-pragma-<wbr>intrinsic">;<br>
 def UnknownPragmas : DiagGroup<"unknown-pragmas">;<br>
-def IgnoredPragmas : DiagGroup<"ignored-pragmas">;<br>
+def IgnoredPragmas : DiagGroup<"ignored-pragmas", [IgnoredPragmaIntrinsic]>;<br>
 def Pragmas : DiagGroup<"pragmas", [UnknownPragmas, IgnoredPragmas]>;<br>
 def UnknownWarningOption : DiagGroup<"unknown-warning-<wbr>option">;<br>
 def NSobjectAttribute : DiagGroup<"NSObject-attribute"<wbr>>;<br>
Index: test/Preprocessor/pragma_<wbr>microsoft.c<br>
==============================<wbr>==============================<wbr>=======<br>
--- test/Preprocessor/pragma_<wbr>microsoft.c<br>
+++ test/Preprocessor/pragma_<wbr>microsoft.c<br>
@@ -178,3 +178,15 @@<br>
 #pragma intrinsic(memset) // no-warning<br>
 #undef __INTRIN_H<br>
 #pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including <intrin.h>}}<br>
+<br>
+#pragma clang diagnostic push<br>
+#pragma clang diagnostic ignored "-Wignored-pragma-intrinsic"<br>
+#pragma intrinsic(asdf) // no-warning<br>
+#pragma clang diagnostic pop<br>
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including <intrin.h>}}<br>
+<br>
+#pragma clang diagnostic push<br>
+#pragma clang diagnostic ignored "-Wignored-pragmas"<br>
+#pragma intrinsic(asdf) // no-warning<br>
+#pragma clang diagnostic pop<br>
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including <intrin.h>}}<br>
<br>
<br>
</blockquote></div><br></div>