[cfe-commits] [PATCH] Fixed a problem with #pragma push_macro/pop_macro implementation. Added tests.

[Alexander Kornienko]

Hi doug.gregor, klimek,

The problem was with the following sequence:
  #pragma push_macro("long")
  #undef long
  #pragma pop_macro("long")
in case when "long" didn't represent a macro.
Fixed crash and removed code duplication for #undef/pop_macro case.

http://llvm-reviews.chandlerc.com/D31

Files:
  tools/clang/test/Preprocessor/pragma-pushpop-macro.c
  tools/clang/include/clang/Lex/Preprocessor.h
  tools/clang/lib/Lex/PPDirectives.cpp
  tools/clang/lib/Lex/Pragma.cpp
  tools/clang/lib/Lex/PPMacroExpansion.cpp

Index: tools/clang/test/Preprocessor/pragma-pushpop-macro.c
===================================================================
--- tools/clang/test/Preprocessor/pragma-pushpop-macro.c
+++ tools/clang/test/Preprocessor/pragma-pushpop-macro.c
@@ -31,11 +31,28 @@
 #define Y 4
 int pmy2 = Y;
 
+// The sequence push, define/undef, pop caused problems if macro was not
+// previously defined.
+#pragma push_macro("PREVIOUSLY_UNDEFINED1")
+#undef PREVIOUSLY_UNDEFINED1
+#pragma pop_macro("PREVIOUSLY_UNDEFINED1")
+#ifndef PREVIOUSLY_UNDEFINED1
+int Q;
+#endif
+
+#pragma push_macro("PREVIOUSLY_UNDEFINED2")
+#define PREVIOUSLY_UNDEFINED2
+#pragma pop_macro("PREVIOUSLY_UNDEFINED2")
+#ifndef PREVIOUSLY_UNDEFINED2
+int P;
+#endif
+
 // CHECK: int pmx0 = 1
 // CHECK: int pmy0 = 2
 // CHECK: int pmx1 = 1
 // CHECK: int pmx2 = 2
 // CHECK: int pmx3 = 1
 // CHECK: int pmy1 = 3
 // CHECK: int pmy2 = 4
-
+// CHECK: int Q;
+// CHECK: int P;
Index: tools/clang/include/clang/Lex/Preprocessor.h
===================================================================
--- tools/clang/include/clang/Lex/Preprocessor.h
+++ tools/clang/include/clang/Lex/Preprocessor.h
@@ -470,6 +470,8 @@
   /// \brief Specify a macro for this identifier.
   void setMacroInfo(IdentifierInfo *II, MacroInfo *MI,
                     bool LoadedFromAST = false);
+  /// \brief Undefine a macro for this identifier.
+  void clearMacroInfo(IdentifierInfo *II, bool LoadedFromAST = false);
 
   /// macro_iterator/macro_begin/macro_end - This allows you to walk the macro
   /// history table. Currently defined macros have
Index: tools/clang/lib/Lex/PPDirectives.cpp
===================================================================
--- tools/clang/lib/Lex/PPDirectives.cpp
+++ tools/clang/lib/Lex/PPDirectives.cpp
@@ -1921,10 +1921,7 @@
     WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
 
   MI->setUndefLoc(MacroNameTok.getLocation());
-  IdentifierInfo *II = MacroNameTok.getIdentifierInfo();
-  II->setHasMacroDefinition(false);
-  if (II->isFromAST())
-    II->setChangedSinceDeserialization();
+  clearMacroInfo(MacroNameTok.getIdentifierInfo());
 }
 
 
Index: tools/clang/lib/Lex/Pragma.cpp
===================================================================
--- tools/clang/lib/Lex/Pragma.cpp
+++ tools/clang/lib/Lex/Pragma.cpp
@@ -737,13 +737,18 @@
     if (MacroInfo *CurrentMI = getMacroInfo(IdentInfo)) {
       if (CurrentMI->isWarnIfUnused())
         WarnUnusedMacroLocs.erase(CurrentMI->getDefinitionLoc());
+      CurrentMI->setUndefLoc(MessageLoc);
     }
 
     // Get the MacroInfo we want to reinstall.
     MacroInfo *MacroToReInstall = iter->second.back();
 
-    // Reinstall the previously pushed macro.
-    setMacroInfo(IdentInfo, MacroToReInstall);
+    if (MacroToReInstall) {
+      // Reinstall the previously pushed macro.
+      setMacroInfo(IdentInfo, MacroToReInstall);
+    } else if (IdentInfo->hasMacroDefinition()) {
+      clearMacroInfo(IdentInfo);
+    }
 
     // Pop PragmaPushMacroInfo stack.
     iter->second.pop_back();
Index: tools/clang/lib/Lex/PPMacroExpansion.cpp
===================================================================
--- tools/clang/lib/Lex/PPMacroExpansion.cpp
+++ tools/clang/lib/Lex/PPMacroExpansion.cpp
@@ -57,6 +57,15 @@
     II->setChangedSinceDeserialization();
 }
 
+/// \brief Undefine a macro for this identifier.
+void Preprocessor::clearMacroInfo(IdentifierInfo *II, bool LoadedFromAST) {
+  assert(II->hasMacroDefinition() && "Macro is not defined!");
+  assert(Macros[II]->getUndefLoc().isValid() && "Macro is still defined!");
+  II->setHasMacroDefinition(false);
+  if (II->isFromAST() && !LoadedFromAST)
+    II->setChangedSinceDeserialization();
+}
+
 /// RegisterBuiltinMacro - Register the specified identifier in the identifier
 /// table and mark it as a builtin macro to be expanded.
 static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31.1.patch
Type: text/x-patch
Size: 3927 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120828/64ea761c/attachment.bin>