[cfe-dev] [PATCH] preprocessor pragma push_macro support

[Michael Spencer]

Overall the patch looks good. HandlePragmaPushMacro and
HandlePragmaPopMacro seem to share a lot of code and you may consider
refactoring the parsing and lookup into another function. There are
some things that I'm not sure about and will have to be checked by
others.

It passes the test I wrote while going through the MS extensions. I've
attached that test which you should add to your patch.

One other thing is that clang currently warns about macro
redefinitions after a push. It would be nice if it didn't on the first
redefinition after a push, as you pushed it so you could redefine it
:P.

Now for some inline comments.

>Index: include/clang/Basic/DiagnosticLexKinds.td
>===================================================================
>--- include/clang/Basic/DiagnosticLexKinds.td	(revision 109329)
>+++ include/clang/Basic/DiagnosticLexKinds.td	(working copy)
>@@ -230,6 +230,12 @@
>   "pragma comment requires parenthesized identifier and optional string">;
> def err_pragma_message_malformed : Error<
>   "pragma message requires parenthesized string">;
>+def err_pragma_push_macro_malformed : Error<
>+   "pragma push_macro requires a parenthesized string">;
>+def err_pragma_pop_macro_malformed : Error<
>+   "pragma pop_macro requires a parenthesized string">;
>+def warn_pragma_pop_macro_no_push : Warning<
>+   "#pragma pop_macro : '%0' no previous #pragma push_macro for this identifier">;

This may be better worded as "pragma pop_macro could not pop '%0', no
matching push_macro", but that's really up to personal preference and
whatnot. I just matched what warn_pragma_diagnostic_clang_cannot_ppp
says for consistency.

> def warn_pragma_message : Warning<"%0">;
> def warn_pragma_ignored : Warning<"unknown pragma ignored">,
>    InGroup<UnknownPragmas>, DefaultIgnore;
>Index: lib/Lex/Pragma.cpp
>===================================================================
>--- lib/Lex/Pragma.cpp	(revision 109329)
>+++ lib/Lex/Pragma.cpp	(working copy)
>@@ -16,6 +16,7 @@
> #include "clang/Lex/HeaderSearch.h"
> #include "clang/Lex/LiteralSupport.h"
> #include "clang/Lex/Preprocessor.h"
>+#include "clang/Lex/MacroInfo.h"
> #include "clang/Lex/LexDiagnostic.h"
> #include "clang/Basic/FileManager.h"
> #include "clang/Basic/SourceManager.h"
>@@ -483,7 +484,131 @@
>     Callbacks->PragmaMessage(MessageLoc, MessageString);
> }
>
>+/// HandlePragmaPushMacro - Handle #pragma push_macro.
>+/// The syntax is:
>+///   #pragma push_macro("macro")
>+void Preprocessor::HandlePragmaPushMacro(Token &PushMacroTok) {
>+  // Remember the pragma token location.
>+  SourceLocation PragmaLoc = PushMacroTok.getLocation();
>
>+  // Read the '('.
>+  Token Tok;
>+  Lex(Tok);
>+  if (Tok.isNot(tok::l_paren)) {
>+    Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>+    return;
>+  }
>+
>+  // Read the macro name string.
>+  Lex(Tok);
>+  if (Tok.isNot(tok::string_literal)) {
>+    Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>+    return;
>+  }
>+
>+  // Remember the macro string.
>+  std::string StrVal = getSpelling(Tok);

This should be a llvm::StringRef instead of an std::string.

>+
>+  // Read the ')'.
>+  Lex(Tok);
>+  if (Tok.isNot(tok::r_paren)) {
>+    Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>+    return;
>+  }
>+
>+  assert(StrVal[0] == '"' && StrVal[StrVal.size()-1] == '"' &&
>+         "Invalid string token!");

This should be an actual diagnostic instead of an assert. Input
provided by the user should _never_ cause an assert.

>+
>+  // Create a Token from the string.
>+  Token MacroToPushTok;
>+  MacroToPushTok.startToken();
>+  MacroToPushTok.setKind(tok::identifier);
>+  CreateString(&StrVal[1], StrVal.size() - 2, MacroToPushTok);
>+
>+  // Get the IdentifierInfo of MacroToPushTok.
>+  IdentifierInfo *IdentInfo = LookUpIdentifierInfo(MacroToPushTok);
>+
>+  // Get the MacroInfo and flag IsPragmaPush.
>+  MacroInfo *MI = getMacroInfo(IdentInfo);
>+  if (MI) {
>+    MI->setIsPragmaPush(true);
>+  }
>+
>+  // Push the MacroInfo pointer so we can retrieve in later.
>+  PragmaPushMacroInfo[IdentInfo].push(MI);
>+}
>+
>+/// HandlePragmaPopMacro - Handle #pragma push_macro.
>+/// The syntax is:
>+///   #pragma pop_macro("macro")
>+void Preprocessor::HandlePragmaPopMacro(Token &PopMacroTok) {
>+  SourceLocation PragmaLoc = PopMacroTok.getLocation();
>+  Token Tok;
>+
>+  // Read the '('.
>+  Lex(Tok);
>+  if (Tok.isNot(tok::l_paren)) {
>+    Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>+    return;
>+  }
>+
>+  // Read the '"..."'.
>+  Lex(Tok);
>+  if (Tok.isNot(tok::string_literal)) {
>+    Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>+    return;
>+  }
>+
>+  // Remember the string.
>+  std::string StrVal = getSpelling(Tok);

This should be a llvm::StringRef instead of an std::string.

>+
>+  // Read the ')'.
>+  Lex(Tok);
>+  if (Tok.isNot(tok::r_paren)) {
>+    Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>+    return;
>+  }
>+
>+  assert(StrVal[0] == '"' && StrVal[StrVal.size()-1] == '"' &&
>+         "Invalid string token!");

This should be an actual diagnostic instead of an assert. Input
provided by the user should _never_ cause an assert.

>+
>+  // Create a Token from the string.
>+  Token MacroToPopTok;
>+  MacroToPopTok.startToken();
>+  CreateString(&StrVal[1], StrVal.size() - 2, MacroToPopTok);
>+  MacroToPopTok.setKind(tok::identifier);
>+
>+  // Get the IdentifierInfo of MacroToPopTok.
>+  IdentifierInfo *IdentInfo = LookUpIdentifierInfo(MacroToPopTok);
>+
>+  // Retrived the stack<MacroInfo*> associated with the macro.
>+  std::map<IdentifierInfo*, std::stack<MacroInfo*> >::iterator iter =
>+    PragmaPushMacroInfo.find(IdentInfo);
>+  if (iter != PragmaPushMacroInfo.end()) {
>+    // PushedMI is the MacroInfo will want to reinstall.
>+    MacroInfo *PushedMI = iter->second.top();
>+
>+    // CurrentMI is the current MacroInfo to release.
>+    MacroInfo *CurrentMI = getMacroInfo(IdentInfo);
>+    if (CurrentMI && CurrentMI != PushedMI)
>+      ReleaseMacroInfo(CurrentMI);
>+
>+    // Reinstall the pushed macro.
>+    setMacroInfo(IdentInfo, PushedMI);
>+
>+    // Reset the IsPragmaPush flag to false.
>+    if (PushedMI) PushedMI->setIsPragmaPush(false);
>+
>+    // Pop PragmaPushMacroInfo stack.
>+    iter->second.pop();
>+    if (iter->second.size() == 0)
>+      PragmaPushMacroInfo.erase(iter);
>+  }
>+  else {

No newline before the else.

>+    Diag(PragmaLoc, diag::warn_pragma_pop_macro_no_push) << getSpelling(MacroToPopTok);

Keep lines below 80 columns.

>+  }
>+}
>+
> /// AddPragmaHandler - Add the specified pragma handler to the preprocessor.
> /// If 'Namespace' is non-null, then it is a token required to exist on the
> /// pragma line before the pragma string starts, e.g. "STDC" or "GCC".
>@@ -699,6 +824,23 @@
>   }
> };
>
>+/// PragmaPushMacroHandler - "#pragma push_macro" marks the file as atomically included.

Keep lines below 80 columns. And the comment is wrong.

>+struct PragmaPushMacroHandler : public PragmaHandler {
>+  PragmaPushMacroHandler() : PragmaHandler("push_macro") {}
>+  virtual void HandlePragma(Preprocessor &PP, Token &PushMacroTok) {
>+    PP.HandlePragmaPushMacro(PushMacroTok);
>+  }
>+};
>+
>+
>+/// PragmaPopMacroHandler - "#pragma pop_macro" marks the file as atomically included.

Keep lines below 80 columns. And the comment is wrong.

>+struct PragmaPopMacroHandler : public PragmaHandler {
>+  PragmaPopMacroHandler() : PragmaHandler("pop_macro") {}
>+  virtual void HandlePragma(Preprocessor &PP, Token &PopMacroTok) {
>+    PP.HandlePragmaPopMacro(PopMacroTok);
>+  }
>+};
>+
> // Pragma STDC implementations.
>
> enum STDCSetting {
>@@ -780,6 +922,8 @@
> void Preprocessor::RegisterBuiltinPragmas() {
>   AddPragmaHandler(new PragmaOnceHandler());
>   AddPragmaHandler(new PragmaMarkHandler());
>+  AddPragmaHandler(new PragmaPushMacroHandler());
>+  AddPragmaHandler(new PragmaPopMacroHandler());
>
>   // #pragma GCC ...
>   AddPragmaHandler("GCC", new PragmaPoisonHandler());

- Michael Spencer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pragma-pushpop-macro.c
Type: application/octet-stream
Size: 825 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100727/ec261984/attachment.obj>