alexshap updated this revision to Diff 101962.
alexshap added a comment.
Address comments
Repository:
rL LLVM
https://reviews.llvm.org/D33976
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
@@ -0,0 +1,59 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -Wformat -fixit %t
+// RUN: grep -v CHECK %t | FileCheck %s
+
+/* This is a test of code modifications created by darwin format fix-its hints
+ that are provided as part of warning */
+
+int printf(const char * restrict, ...);
+
+#if __LP64__
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+#else
+typedef int NSInteger;
+typedef unsigned int NSUInteger;
+#endif
+NSInteger getNSInteger();
+NSUInteger getNSUInteger();
+
+#define Log1(...) \
+do { \
+ printf(__VA_ARGS__); \
+} while (0)
+
+#define Log2(...) \
+do { \
+ printf(__VA_ARGS__); \
+ printf(__VA_ARGS__); \
+} while (0) \
+
+#define Log3(X, Y, Z) \
+do { \
+ printf(X, Y); \
+ printf(X, Z); \
+} while (0) \
+
+void test() {
+ printf("test 1: %s", getNSInteger());
+ // CHECK: printf("test 1: %ld", (long)getNSInteger());
+ printf("test 2: %s %s", getNSInteger(), getNSInteger());
+ // CHECK: printf("test 2: %ld %ld", (long)getNSInteger(), (long)getNSInteger());
+
+ Log1("test 3: %s", getNSInteger());
+ // CHECK: Log1("test 3: %ld", (long)getNSInteger());
+ Log1("test 4: %s %s", getNSInteger(), getNSInteger());
+ // CHECK: Log1("test 4: %ld %ld", (long)getNSInteger(), (long)getNSInteger());
+
+ Log2("test 5: %s", getNSInteger());
+ // CHECK: Log2("test 5: %ld", (long)getNSInteger());
+ Log2("test 6: %s %s", getNSInteger(), getNSInteger());
+ // CHECK: Log2("test 6: %ld %ld", (long)getNSInteger(), (long)getNSInteger());
+
+ // Artificial test to check that X (in Log3(X, Y, Z))
+ // is modified only according to the diagnostics
+ // for the first printf and the modification caused
+ // by the second printf is dropped.
+ Log3("test 7: %s", getNSInteger(), getNSUInteger());
+ // CHECK: Log3("test 7: %ld", (long)getNSInteger(), (unsigned long)getNSUInteger());
+}
Index: lib/Edit/EditedSource.cpp
===================================================================
--- lib/Edit/EditedSource.cpp
+++ lib/Edit/EditedSource.cpp
@@ -25,30 +25,28 @@
void EditedSource::deconstructMacroArgLoc(SourceLocation Loc,
SourceLocation &ExpansionLoc,
- IdentifierInfo *&II) {
+ MacroArgUse &ArgUse) {
assert(SourceMgr.isMacroArgExpansion(Loc));
SourceLocation DefArgLoc = SourceMgr.getImmediateExpansionRange(Loc).first;
ExpansionLoc = SourceMgr.getImmediateExpansionRange(DefArgLoc).first;
SmallString<20> Buf;
StringRef ArgName = Lexer::getSpelling(SourceMgr.getSpellingLoc(DefArgLoc),
Buf, SourceMgr, LangOpts);
- II = nullptr;
- if (!ArgName.empty()) {
- II = &IdentTable.get(ArgName);
- }
+ ArgUse = {nullptr, SourceLocation()};
+ if (!ArgName.empty())
+ ArgUse = {&IdentTable.get(ArgName), SourceMgr.getSpellingLoc(DefArgLoc)};
}
void EditedSource::startingCommit() {}
void EditedSource::finishedCommit() {
for (auto &ExpArg : CurrCommitMacroArgExps) {
SourceLocation ExpLoc;
- IdentifierInfo *II;
- std::tie(ExpLoc, II) = ExpArg;
- auto &ArgNames = ExpansionToArgMap[ExpLoc.getRawEncoding()];
- if (std::find(ArgNames.begin(), ArgNames.end(), II) == ArgNames.end()) {
- ArgNames.push_back(II);
- }
+ MacroArgUse ArgUse;
+ std::tie(ExpLoc, ArgUse) = ExpArg;
+ auto &ArgUses = ExpansionToArgMap[ExpLoc.getRawEncoding()];
+ if (std::find(ArgUses.begin(), ArgUses.end(), ArgUse) == ArgUses.end())
+ ArgUses.push_back(ArgUse);
}
CurrCommitMacroArgExps.clear();
}
@@ -66,12 +64,15 @@
}
if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
- IdentifierInfo *II;
SourceLocation ExpLoc;
- deconstructMacroArgLoc(OrigLoc, ExpLoc, II);
+ MacroArgUse ArgUse;
+ deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
auto I = ExpansionToArgMap.find(ExpLoc.getRawEncoding());
if (I != ExpansionToArgMap.end() &&
- std::find(I->second.begin(), I->second.end(), II) != I->second.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()) {
// 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:
@@ -101,11 +102,11 @@
return true;
if (SourceMgr.isMacroArgExpansion(OrigLoc)) {
- IdentifierInfo *II;
SourceLocation ExpLoc;
- deconstructMacroArgLoc(OrigLoc, ExpLoc, II);
- if (II)
- CurrCommitMacroArgExps.emplace_back(ExpLoc, II);
+ MacroArgUse ArgUse;
+ deconstructMacroArgLoc(OrigLoc, ExpLoc, ArgUse);
+ if (ArgUse.first)
+ CurrCommitMacroArgExps.emplace_back(ExpLoc, ArgUse);
}
FileEdit &FA = FileEdits[Offs];
Index: include/clang/Edit/EditedSource.h
===================================================================
--- include/clang/Edit/EditedSource.h
+++ include/clang/Edit/EditedSource.h
@@ -41,9 +41,11 @@
typedef std::map<FileOffset, FileEdit> FileEditsTy;
FileEditsTy FileEdits;
- llvm::DenseMap<unsigned, llvm::TinyPtrVector<IdentifierInfo*>>
+ // Location of argument use inside the macro body
+ typedef std::pair<IdentifierInfo*, SourceLocation> MacroArgUse;
+ llvm::DenseMap<unsigned, SmallVector<MacroArgUse, 2>>
ExpansionToArgMap;
- SmallVector<std::pair<SourceLocation, IdentifierInfo*>, 2>
+ SmallVector<std::pair<SourceLocation, MacroArgUse>, 2>
CurrCommitMacroArgExps;
IdentifierTable IdentTable;
@@ -84,7 +86,7 @@
FileEditsTy::iterator getActionForOffset(FileOffset Offs);
void deconstructMacroArgLoc(SourceLocation Loc,
SourceLocation &ExpansionLoc,
- IdentifierInfo *&II);
+ MacroArgUse &ArgUse);
void startingCommit();
void finishedCommit();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
