[PATCH] D34268: [clang] Fix format specifiers fixits for nested macros

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap updated this revision to Diff 102781.
alexshap added a comment.

minor update


Repository:
  rL LLVM

https://reviews.llvm.org/D34268

Files:
  include/clang/Edit/EditedSource.h
  lib/Edit/EditedSource.cpp
  test/FixIt/fixit-format-darwin.m


Index: test/FixIt/fixit-format-darwin.m
===================================================================
--- test/FixIt/fixit-format-darwin.m
+++ test/FixIt/fixit-format-darwin.m
@@ -57,3 +57,20 @@
   Log3("test 7: %s", getNSInteger(), getNSUInteger());
   // CHECK: Log3("test 7: %ld", (long)getNSInteger(), (unsigned long)getNSUInteger());
 }
+
+#define Outer1(...) \
+do { \
+  printf(__VA_ARGS__); \
+} while (0)
+
+#define Outer2(...) \
+do { \
+  Outer1(__VA_ARGS__); Outer1(__VA_ARGS__); \
+} while (0)
+
+void bug33447() {
+  Outer2("test 8: %s", getNSInteger());  
+  // CHECK: Outer2("test 8: %ld", (long)getNSInteger());
+  Outer2("test 9: %s %s", getNSInteger(), getNSInteger());
+  // CHECK: Outer2("test 9: %ld %ld", (long)getNSInteger(), (long)getNSInteger());
+}
Index: lib/Edit/EditedSource.cpp
===================================================================
--- lib/Edit/EditedSource.cpp
+++ lib/Edit/EditedSource.cpp
@@ -24,17 +24,22 @@
 }
 
 void EditedSource::deconstructMacroArgLoc(SourceLocation Loc,
-                                          SourceLocation &ExpansionLoc,
                                           MacroArgUse &ArgUse) {
   assert(SourceMgr.isMacroArgExpansion(Loc));
   SourceLocation DefArgLoc = SourceMgr.getImmediateExpansionRange(Loc).first;
-  ExpansionLoc = SourceMgr.getImmediateExpansionRange(DefArgLoc).first;
+  SmallVector<SourceLocation, 1> ExpansionStack;
+  ExpansionStack.push_back(
+      SourceMgr.getImmediateExpansionRange(DefArgLoc).first);
+  while (SourceMgr.isMacroBodyExpansion(ExpansionStack.back()))
+    ExpansionStack.push_back(
+        SourceMgr.getImmediateExpansionRange(ExpansionStack.back()).first);
   SmallString<20> Buf;
   StringRef ArgName = Lexer::getSpelling(SourceMgr.getSpellingLoc(DefArgLoc),
                                          Buf, SourceMgr, LangOpts);
-  ArgUse = {nullptr, SourceLocation()};
+  ArgUse = MacroArgUse{nullptr, {}, SourceLocation()};
   if (!ArgName.empty())
-    ArgUse = {&IdentTable.get(ArgName), SourceMgr.getSpellingLoc(DefArgLoc)};
+    ArgUse = {&IdentTable.get(ArgName), std::move(ExpansionStack),
+              SourceMgr.getSpellingLoc(DefArgLoc)};
 }
 
 void EditedSource::startingCommit() {}
@@ -64,15 +69,17 @@
   }
 
   if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
-    SourceLocation ExpLoc;
     MacroArgUse ArgUse;
-    deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
+    deconstructMacroArgLoc(OrigLoc, ArgUse);
+    assert(!ArgUse.ExpansionStack.empty());
+    SourceLocation ExpLoc = ArgUse.ExpansionStack.back();
     auto I = ExpansionToArgMap.find(ExpLoc.getRawEncoding());
     if (I != ExpansionToArgMap.end() &&
-        std::find_if(
-            I->second.begin(), I->second.end(), [&](const MacroArgUse &U) {
-              return ArgUse.first == U.first && ArgUse.second != U.second;
-            }) != I->second.end()) {
+        find_if(I->second, [&](const MacroArgUse &U) {
+          return ArgUse.Identifier == U.Identifier &&
+                 std::tie(ArgUse.ExpansionStack, ArgUse.Use) !=
+                     std::tie(U.ExpansionStack, U.Use);
+        }) != I->second.end()) {
       // Trying to write in a macro argument input that has already been
       // written by a previous commit for another expansion of the same macro
       // argument name. For example:
@@ -89,7 +96,6 @@
       return false;
     }
   }
-
   return true;
 }
 
@@ -102,13 +108,14 @@
     return true;
 
   if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
-    SourceLocation ExpLoc;
     MacroArgUse ArgUse;
-    deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
-    if (ArgUse.first)
-      CurrCommitMacroArgExps.emplace_back(ExpLoc, ArgUse);
+    deconstructMacroArgLoc(OrigLoc, ArgUse);
+    if (ArgUse.Identifier) {
+      assert(!ArgUse.ExpansionStack.empty());
+      CurrCommitMacroArgExps.emplace_back(ArgUse.ExpansionStack.back(), ArgUse);
+    }
   }
-  
+
   FileEdit &FA = FileEdits[Offs];
   if (FA.Text.empty()) {
     FA.Text = copyString(text);
Index: include/clang/Edit/EditedSource.h
===================================================================
--- include/clang/Edit/EditedSource.h
+++ include/clang/Edit/EditedSource.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/Allocator.h"
 #include <map>
+#include <tuple>
 
 namespace clang {
   class LangOptions;
@@ -41,10 +42,20 @@
   typedef std::map<FileOffset, FileEdit> FileEditsTy;
   FileEditsTy FileEdits;
 
-  // Location of argument use inside the macro body 
-  typedef std::pair<IdentifierInfo*, SourceLocation> MacroArgUse;
-  llvm::DenseMap<unsigned, SmallVector<MacroArgUse, 2>>
-    ExpansionToArgMap;
+  struct MacroArgUse {
+    IdentifierInfo *Identifier;
+    // Stack of macro expansions at given point of source code
+    SmallVector<SourceLocation, 1> ExpansionStack;
+    // Location of argument use inside the top-level macro
+    SourceLocation Use;
+
+    bool operator==(const MacroArgUse &Other) const {
+      return std::tie(Identifier, ExpansionStack, Use) ==
+             std::tie(Other.Identifier, Other.ExpansionStack, Other.Use);
+    }
+  };
+
+  llvm::DenseMap<unsigned, SmallVector<MacroArgUse, 2>> ExpansionToArgMap;
   SmallVector<std::pair<SourceLocation, MacroArgUse>, 2>
     CurrCommitMacroArgExps;
 
@@ -85,7 +96,6 @@
                           bool &Invalid);
   FileEditsTy::iterator getActionForOffset(FileOffset Offs);
   void deconstructMacroArgLoc(SourceLocation Loc,
-                              SourceLocation &ExpansionLoc,
                               MacroArgUse &ArgUse);
 
   void startingCommit();

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits