Parsing MS pragma intrinsic

[agutowski]

Summary

Parse pragma intrinsic, display warning if the function isn't a builtin function in clang and suggest including intrin.h.

[rnk]

include/clang/Basic/DiagnosticParseKinds.td

916 ↗

(On Diff #69430)

I'd use this wording:

'%0' is not a recognized builtin

919 ↗

(On Diff #69430)

Generally notes are attached to other source locations, which we don't have in this case. I'd replace this note with a second diagnostic with an extra clause:

def warn_pragma_intrinsic_builtin_suggest : Warning<
  "'%0' is not a recognized builtin; consider including <intrin.h> to access non-builtin intrinsics">;

If you search for "suggest" in the diagnostic file, you can see other instances of this.

[agutowski]

Changed warning messages

[aaron.ballman]

include/clang/Basic/DiagnosticParseKinds.td

918 ↗

(On Diff #69605)

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".

lib/Parse/ParsePragma.cpp

2148 ↗

(On Diff #69605)

• tells the compiler
• of the function

2150 ↗

(On Diff #69605)

pragma -> #pragma

2157 ↗

(On Diff #69605)

Not for your patch, but I wish we could use the balanced delimiter tracker for parsing pragmas.

2164 ↗

(On Diff #69605)

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?

2169–2174 ↗

(On Diff #69605)

Should elide braces here.

[agutowski]

lib/Parse/ParsePragma.cpp

2164 ↗

(On Diff #69605)

I guess we only care about intrin.h from Clang - as far as I can see, MSVC's intrin.h contains only declarations, so including it won't change anything most of the time if the intrinsic isn't implemented in Clang.

[agutowski]

Changed warning template to use select, fixed some mistakes in the comments.

[aaron.ballman]

LGTM, but you should wait for @rnk to accept as well.

[rnk]

One buglet, otherwise looks good.

lib/Parse/ParsePragma.cpp

2169 ↗

(On Diff #69632)

This doesn't seem like the right test. I think '1' is a valid builtin. This should probably be != 0. __builtin_atan2 looks like it has builtin id 1, and you can use that to test.

[agutowski]

Fixed checking if the function is an intrinsic. Updated getBuiltinID description.

[agutowski]

Fixed typo

[rnk]

lgtm

cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td

def warn_pragma_expected_action_or_r_paren : Warning<
   "expected action or ')' in '#pragma %0' - ignored">,
   InGroup<IgnoredPragmas>;
 def warn_pragma_invalid_action : Warning<
   "unknown action for '#pragma %0' - ignored">,
   InGroup<IgnoredPragmas>;
 def warn_pragma_pack_malformed : Warning<
   "expected integer or identifier in '#pragma pack' - ignored">,
   InGroup<IgnoredPragmas>;
+// - #pragma intrinsic
+def warn_pragma_intrinsic_builtin : Warning<
+  "%0 is not a recognized builtin%select{|; consider including <intrin.h> to access non-builtin intrinsics}1">,
+  InGroup<IgnoredPragmas>;
 // - #pragma unused
 def warn_pragma_unused_expected_var : Warning<
   "expected '#pragma unused' argument to be a variable name">,
   InGroup<IgnoredPragmas>;
 // - #pragma init_seg
 def warn_pragma_init_seg_unsupported_target : Warning<
   "'#pragma init_seg' is only supported when targeting a "
   "Microsoft environment">,

cfe/trunk/include/clang/Basic/IdentifierTable.h

public:
   /// the name of a known builtin library function is used to declare that
   /// function, but an unexpected type is specified.
   void revertBuiltin() {
     setBuiltinID(0);
   }
 
   /// \brief Return a value indicating whether this is a builtin function.
   ///
-  /// 0 is not-built-in.  1 is builtin-for-some-nonprimary-target.
-  /// 2+ are specific builtin functions.
+  /// 0 is not-built-in. 1+ are specific builtin functions.
   unsigned getBuiltinID() const {
     if (ObjCOrBuiltinID >= tok::NUM_OBJC_KEYWORDS)
       return ObjCOrBuiltinID - tok::NUM_OBJC_KEYWORDS;
     else
       return 0;
   }
   void setBuiltinID(unsigned ID) {
     ObjCOrBuiltinID = ID + tok::NUM_OBJC_KEYWORDS;

cfe/trunk/include/clang/Parse/Parser.h

class Parser : public CodeCompletionHandler {
   std::unique_ptr<PragmaHandler> MSVtorDisp;
   std::unique_ptr<PragmaHandler> MSInitSeg;
   std::unique_ptr<PragmaHandler> MSDataSeg;
   std::unique_ptr<PragmaHandler> MSBSSSeg;
   std::unique_ptr<PragmaHandler> MSConstSeg;
   std::unique_ptr<PragmaHandler> MSCodeSeg;
   std::unique_ptr<PragmaHandler> MSSection;
   std::unique_ptr<PragmaHandler> MSRuntimeChecks;
+  std::unique_ptr<PragmaHandler> MSIntrinsic;
   std::unique_ptr<PragmaHandler> OptimizeHandler;
   std::unique_ptr<PragmaHandler> LoopHintHandler;
   std::unique_ptr<PragmaHandler> UnrollHintHandler;
   std::unique_ptr<PragmaHandler> NoUnrollHintHandler;
 
   std::unique_ptr<CommentHandler> CommentSemaHandler;
 
   /// Whether the '>' token acts as an operator or not. This will be

cfe/trunk/lib/Parse/ParsePragma.cpp

struct PragmaUnrollHintHandler : public PragmaHandler {
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
                     Token &FirstToken) override;
 };
 
 struct PragmaMSRuntimeChecksHandler : public EmptyPragmaHandler {
   PragmaMSRuntimeChecksHandler() : EmptyPragmaHandler("runtime_checks") {}
 };
 
+struct PragmaMSIntrinsicHandler : public PragmaHandler {
+  PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                    Token &FirstToken) override;
+};
+
 }  // end namespace
 
 void Parser::initializePragmaHandlers() {
   AlignHandler.reset(new PragmaAlignHandler());
   PP.AddPragmaHandler(AlignHandler.get());
 
   GCCVisibilityHandler.reset(new PragmaGCCVisibilityHandler());
   PP.AddPragmaHandler("GCC", GCCVisibilityHandler.get());
if (getLangOpts().MicrosoftExt) {
     MSConstSeg.reset(new PragmaMSPragma("const_seg"));
     PP.AddPragmaHandler(MSConstSeg.get());
     MSCodeSeg.reset(new PragmaMSPragma("code_seg"));
     PP.AddPragmaHandler(MSCodeSeg.get());
     MSSection.reset(new PragmaMSPragma("section"));
     PP.AddPragmaHandler(MSSection.get());
     MSRuntimeChecks.reset(new PragmaMSRuntimeChecksHandler());
     PP.AddPragmaHandler(MSRuntimeChecks.get());
+    MSIntrinsic.reset(new PragmaMSIntrinsicHandler());
+    PP.AddPragmaHandler(MSIntrinsic.get());
   }
 
   OptimizeHandler.reset(new PragmaOptimizeHandler(Actions));
   PP.AddPragmaHandler("clang", OptimizeHandler.get());
 
   LoopHintHandler.reset(new PragmaLoopHintHandler());
   PP.AddPragmaHandler("clang", LoopHintHandler.get());
 
if (getLangOpts().MicrosoftExt) {
     PP.RemovePragmaHandler(MSConstSeg.get());
     MSConstSeg.reset();
     PP.RemovePragmaHandler(MSCodeSeg.get());
     MSCodeSeg.reset();
     PP.RemovePragmaHandler(MSSection.get());
     MSSection.reset();
     PP.RemovePragmaHandler(MSRuntimeChecks.get());
     MSRuntimeChecks.reset();
+    PP.RemovePragmaHandler(MSIntrinsic.get());
+    MSIntrinsic.reset();
   }
 
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
   FPContractHandler.reset();
 
   PP.RemovePragmaHandler("clang", OptimizeHandler.get());
   OptimizeHandler.reset();
 
void PragmaUnrollHintHandler::HandlePragma(Preprocessor &PP,
   TokenArray[0].startToken();
   TokenArray[0].setKind(tok::annot_pragma_loop_hint);
   TokenArray[0].setLocation(PragmaName.getLocation());
   TokenArray[0].setAnnotationEndLoc(PragmaName.getLocation());
   TokenArray[0].setAnnotationValue(static_cast<void *>(Info));
   PP.EnterTokenStream(std::move(TokenArray), 1,
                       /*DisableMacroExpansion=*/false);
 }
+
+/// \brief Handle the Microsoft \#pragma intrinsic extension.
+///
+/// The syntax is:
+/// \code
+///  #pragma intrinsic(memset)
+///  #pragma intrinsic(strlen, memcpy)
+/// \endcode
+///
+/// Pragma intrisic tells the compiler to use a builtin version of the
+/// function. 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.
+void PragmaMSIntrinsicHandler::HandlePragma(Preprocessor &PP,
+                                            PragmaIntroducerKind Introducer,
+                                            Token &Tok) {
+  PP.Lex(Tok);
+
+  if (Tok.isNot(tok::l_paren)) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen)
+        << "intrinsic";
+    return;
+  }
+  PP.Lex(Tok);
+
+  bool SuggestIntrinH = !PP.isMacroDefined("__INTRIN_H");
+
+  while (Tok.is(tok::identifier)) {
+    IdentifierInfo *II = Tok.getIdentifierInfo();
+    if (!II->getBuiltinID())
+      PP.Diag(Tok.getLocation(), diag::warn_pragma_intrinsic_builtin)
+          << II << SuggestIntrinH;
+
+    PP.Lex(Tok);
+    if (Tok.isNot(tok::comma))
+      break;
+    PP.Lex(Tok);
+  }
+
+  if (Tok.isNot(tok::r_paren)) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_rparen)
+        << "intrinsic";
+    return;
+  }
+  PP.Lex(Tok);
+
+  if (Tok.isNot(tok::eod))
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
+        << "intrinsic";
+}

cfe/trunk/test/Preprocessor/pragma_microsoft.c

 #pragma warning(disable 4705) // expected-warning {{expected ':'}}
 #pragma warning(disable : 0) // expected-warning {{expected a warning number}}
 #pragma warning(default 321) // expected-warning {{expected ':'}}
 #pragma warning(asdf : 321) // expected-warning {{expected 'push', 'pop'}}
 #pragma warning(push, -1) // expected-warning {{requires a level between 0 and 4}}
 
 // Test that runtime_checks is parsed but ignored.
 #pragma runtime_checks("sc", restore) // no-warning
+
+// Test pragma intrinsic
+#pragma intrinsic(memset) // no-warning
+#pragma intrinsic(memcpy, strlen, strlen) // no-warning
+#pragma intrinsic() // no-warning
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including <intrin.h>}}
+#pragma intrinsic(main) // expected-warning {{'main' is not a recognized builtin; consider including <intrin.h>}}
+#pragma intrinsic( // expected-warning {{missing ')' after}}
+#pragma intrinsic(int) // expected-warning {{missing ')' after}}
+#pragma intrinsic(strcmp) asdf // expected-warning {{extra tokens at end}}
+
+#define __INTRIN_H  // there should be no notes after defining __INTRIN_H
+#pragma intrinsic(asdf) // expected-warning-re {{'asdf' is not a recognized builtin{{$}}}}
+#pragma intrinsic(memset) // no-warning
+#undef __INTRIN_H
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including <intrin.h>}}