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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305845: [clang] Fix format specifiers fixits for nested 
macros (authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D34268?vs=103093&id=103262#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34268

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


Index: cfe/trunk/include/clang/Edit/EditedSource.h
===================================================================
--- cfe/trunk/include/clang/Edit/EditedSource.h
+++ cfe/trunk/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;
+    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;
 
Index: cfe/trunk/test/FixIt/fixit-format-darwin.m
===================================================================
--- cfe/trunk/test/FixIt/fixit-format-darwin.m
+++ cfe/trunk/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: cfe/trunk/lib/Edit/EditedSource.cpp
===================================================================
--- cfe/trunk/lib/Edit/EditedSource.cpp
+++ cfe/trunk/lib/Edit/EditedSource.cpp
@@ -28,13 +28,18 @@
                                           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 @@
     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 @@
       return false;
     }
   }
-
   return true;
 }
 
@@ -102,13 +107,13 @@
     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);

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