alexshap created this revision.
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 (in the source code) which should be used for grouping
together
the edits. The definition of MacroArgUse is adjusted accordingly: now it's
essentially the stack of expansions plus
the location of argument use inside the top-most macro.
This diff fixes https://bugs.llvm.org/show_bug.cgi?id=33447 .
Test plan: make check-all
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 radar33447() {
+ 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,18 @@
}
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()) {
+ std::find_if(I->second.begin(), I->second.end(),
+ [&](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 +97,6 @@
return false;
}
}
-
return true;
}
@@ -102,13 +109,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
