[clang] Fix format specifiers fixits for nested macros

ExpansionLoc was previously calculated incorrectly in the case of
nested macros expansions. In this diff we build the stack of expansions
where the last one is the actual expansion which should be used
for grouping together the edits.
The definition of MacroArgUse is adjusted accordingly.

Test plan: make check-all

Differential revision: https://reviews.llvm.org/D34268

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@305845 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Edit/EditedSource.h b/include/clang/Edit/EditedSource.h
index 97079142073432ad93b7072dacae0c5aeb6f2ded..d95a0c2be805dfc852283e2d960a4a84a7531671 100644 (file)
--- a/include/clang/Edit/EditedSource.h
+++ b/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 @@ class EditedSource {
   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;
+    SourceLocation ImmediateExpansionLoc;
+    // Location of argument use inside the top-level macro
+    SourceLocation UseLoc;
+
+    bool operator==(const MacroArgUse &Other) const {
+      return std::tie(Identifier, ImmediateExpansionLoc, UseLoc) ==
+             std::tie(Other.Identifier, Other.ImmediateExpansionLoc,
+                      Other.UseLoc);
+    }
+  };
+
+  llvm::DenseMap<unsigned, SmallVector<MacroArgUse, 2>> ExpansionToArgMap;
   SmallVector<std::pair<SourceLocation, MacroArgUse>, 2>
     CurrCommitMacroArgExps;
 

diff --git a/lib/Edit/EditedSource.cpp b/lib/Edit/EditedSource.cpp
index 03d45cf185c9f718c1867951f9f6a3e58fc3c3c6..444d0393cccd4d857590751b26e3e8fff88b415d 100644 (file)
--- a/lib/Edit/EditedSource.cpp
+++ b/lib/Edit/EditedSource.cpp
@@ -28,13 +28,18 @@ void EditedSource::deconstructMacroArgLoc(SourceLocation Loc,
                                           MacroArgUse &ArgUse) {
   assert(SourceMgr.isMacroArgExpansion(Loc));
   SourceLocation DefArgLoc = SourceMgr.getImmediateExpansionRange(Loc).first;
-  ExpansionLoc = SourceMgr.getImmediateExpansionRange(DefArgLoc).first;
+  SourceLocation ImmediateExpansionLoc =
+      SourceMgr.getImmediateExpansionRange(DefArgLoc).first;
+  ExpansionLoc = ImmediateExpansionLoc;
+  while (SourceMgr.isMacroBodyExpansion(ExpansionLoc))
+    ExpansionLoc = SourceMgr.getImmediateExpansionRange(ExpansionLoc).first;
   SmallString<20> Buf;
   StringRef ArgName = Lexer::getSpelling(SourceMgr.getSpellingLoc(DefArgLoc),
                                          Buf, SourceMgr, LangOpts);
-  ArgUse = {nullptr, SourceLocation()};
+  ArgUse = MacroArgUse{nullptr, SourceLocation(), SourceLocation()};
   if (!ArgName.empty())
-    ArgUse = {&IdentTable.get(ArgName), SourceMgr.getSpellingLoc(DefArgLoc)};
+    ArgUse = {&IdentTable.get(ArgName), ImmediateExpansionLoc,
+              SourceMgr.getSpellingLoc(DefArgLoc)};
 }
 
 void EditedSource::startingCommit() {}
@@ -69,10 +74,11 @@ bool EditedSource::canInsertInOffset(SourceLocation OrigLoc, FileOffset Offs) {
     deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
     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.ImmediateExpansionLoc, ArgUse.UseLoc) !=
+                     std::tie(U.ImmediateExpansionLoc, U.UseLoc);
+        }) != 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 +95,6 @@ bool EditedSource::canInsertInOffset(SourceLocation OrigLoc, FileOffset Offs) {
       return false;
     }
   }
-
   return true;
 }
 
@@ -102,13 +107,13 @@ bool EditedSource::commitInsert(SourceLocation OrigLoc,
     return true;
 
   if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
-    SourceLocation ExpLoc;
     MacroArgUse ArgUse;
+    SourceLocation ExpLoc;
     deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
-    if (ArgUse.first)
+    if (ArgUse.Identifier)
       CurrCommitMacroArgExps.emplace_back(ExpLoc, ArgUse);
   }
-  
+
   FileEdit &FA = FileEdits[Offs];
   if (FA.Text.empty()) {
     FA.Text = copyString(text);

diff --git a/test/FixIt/fixit-format-darwin.m b/test/FixIt/fixit-format-darwin.m
index bfc71291a5c3a7523e7964f277132163deb6dcbf..077cc0cf21bcc7dcd77b1230deae2507316368a9 100644 (file)
--- a/test/FixIt/fixit-format-darwin.m
+++ b/test/FixIt/fixit-format-darwin.m
@@ -57,3 +57,20 @@ void test() {
   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());
+}