PPCallbacks::MacroUndefined, change signature and add test.

[marsupial]

Summary

The PPCallbacks::MacroUndefined callback is currently insufficient for clients that need to track the MacroDirectives.
This patch adds an additional argument to PPCallbacks::MacroUndefined that is the undef MacroDirective.

[bruno]

Hi,

Can you attach the patch with context? What are you trying to fix by this change? Please also add a testcase.

[marsupial]

I need notification that a macro has been undefined. It's equally as important as a when it was defined.
I'll happily make a test case but want to make sure this would get in before doing so.

[marsupial]

Added test for un-definition callback.

[marsupial]

ping

[manmanren]

Please update the patch with context: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Thanks,
Manman

unittests/Basic/SourceManagerTest.cpp

251

Can we make this an enum?

[marsupial]

ping

[bruno]

LGTM!

lib/Lex/PPDirectives.cpp

   if (MI->isWarnIfUnused())
     WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
 
-  appendMacroDirective(MacroNameTok.getIdentifierInfo(),
-                       AllocateUndefMacroDirective(MacroNameTok.getLocation()));
+  UndefMacroDirective *MD =
+      AllocateUndefMacroDirective(MacroNameTok.getLocation());
+  appendMacroDirective(MacroNameTok.getIdentifierInfo(), MD);
+
+  if (Callbacks)
+    Callbacks->MacroDefined(MacroNameTok, MD);
 }
 
 //===----------------------------------------------------------------------===//

unittests/Basic/SourceManagerTest.cpp

 struct MacroAction {
   SourceLocation Loc;
   std::string Name;
-  bool isDefinition; // if false, it is expansion.
-  
-  MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-    : Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+  int Kind; // 0 expansion, 1 definition, 2 undefinition
+  MacroAction(SourceLocation Loc, StringRef Name, int K = 0)
+    : Loc(Loc), Name(Name), Kind(K) { }
+
+  bool isExpansion() const { return Kind == 0; }
+  bool isDefinition() const { return Kind == 1; }
+  bool isUnDefinition() const { return Kind == 2; }
 };
 
 class MacroTracker : public PPCallbacks {

                     const MacroDirective *MD) override {
     Macros.push_back(MacroAction(MD->getLocation(),
                                  MacroNameTok.getIdentifierInfo()->getName(),
-                                 true));
+                                 MD->getKind()+1));
   }
   void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
                     SourceRange Range, const MacroArgs *Args) override {
     Macros.push_back(MacroAction(MacroNameTok.getLocation(),
-                                 MacroNameTok.getIdentifierInfo()->getName(),
-                                 false));
+                                 MacroNameTok.getIdentifierInfo()->getName()));
   }
 };
 

 
 TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   const char *header =
-    "#define MACRO_IN_INCLUDE 0\n";
+    "#define MACRO_IN_INCLUDE 0\n"
+    "#define MACRO_DEFINED\n"
+    "#undef MACRO_DEFINED\n";
 
   const char *main =
     "#define M(x) x\n"

   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
-  ASSERT_EQ(9U, Macros.size());
+  ASSERT_EQ(13U, Macros.size());
   // #define M(x) x
-  ASSERT_TRUE(Macros[0].isDefinition);
+  ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);
   // #define INC "/test-header.h"
-  ASSERT_TRUE(Macros[1].isDefinition);
+  ASSERT_TRUE(Macros[1].isDefinition());
   ASSERT_EQ("INC", Macros[1].Name);
   // M expansion in #include M(INC)
-  ASSERT_FALSE(Macros[2].isDefinition);
+  ASSERT_FALSE(Macros[2].isDefinition());
   ASSERT_EQ("M", Macros[2].Name);
   // INC expansion in #include M(INC)
-  ASSERT_FALSE(Macros[3].isDefinition);
+  ASSERT_TRUE(Macros[3].isExpansion());
   ASSERT_EQ("INC", Macros[3].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[4].isDefinition);
+  ASSERT_TRUE(Macros[4].isDefinition());
   ASSERT_EQ("MACRO_IN_INCLUDE", Macros[4].Name);
+  // #define MACRO_DEFINED
+  ASSERT_TRUE(Macros[5].isDefinition());
+  ASSERT_FALSE(Macros[5].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[5].Name);
+  // #undef MACRO_DEFINED
+  ASSERT_FALSE(Macros[6].isDefinition());
+  ASSERT_TRUE(Macros[6].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[6].Name);
   // #define INC2 </test-header.h>
-  ASSERT_TRUE(Macros[5].isDefinition);
-  ASSERT_EQ("INC2", Macros[5].Name);
+  ASSERT_TRUE(Macros[7].isDefinition());
+  ASSERT_EQ("INC2", Macros[7].Name);
   // M expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[6].isDefinition);
-  ASSERT_EQ("M", Macros[6].Name);
+  ASSERT_FALSE(Macros[8].isDefinition());
+  ASSERT_EQ("M", Macros[8].Name);
   // INC2 expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[7].isDefinition);
-  ASSERT_EQ("INC2", Macros[7].Name);
+  ASSERT_TRUE(Macros[9].isExpansion());
+  ASSERT_EQ("INC2", Macros[9].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[8].isDefinition);
-  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[8].Name);
+  ASSERT_TRUE(Macros[10].isDefinition());
+  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[10].Name);
 
   // The INC expansion in #include M(INC) comes before the first
   // MACRO_IN_INCLUDE definition of the included file.

 
   // The INC2 expansion in #include M(INC2) comes before the second
   // MACRO_IN_INCLUDE definition of the included file.
-  EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[7].Loc, Macros[8].Loc));
+  EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[9].Loc, Macros[10].Loc));
 }
 
 #endif