r305018 - [clang] Fix format specifiers fixits
Alexander Shaposhnikov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 8 14:44:45 PDT 2017
Author: alexshap
Date: Thu Jun 8 16:44:45 2017
New Revision: 305018
URL: http://llvm.org/viewvc/llvm-project?rev=305018&view=rev
Log:
[clang] Fix format specifiers fixits
This diff fixes printf "fixits" in the case when there is
a wrapping macro and the format string needs multiple replacements.
In the presence of a macro there is an extra logic in EditedSource.cpp
to handle multiple uses of the same macro argument
(see the old comment inside EditedSource::canInsertInOffset)
which was mistriggerred when the argument was used only once
but required multiple adjustments), as a result the "fixit"
was breaking down the format string
by dropping the second format specifier, i.e.
Log1("test 4: %s %s", getNSInteger(), getNSInteger())
was getting replaced with
Log1("test 4: %ld ", (long)getNSInteger(), (long)getNSInteger())
(if one removed the macro and used printf directly it would work fine).
In this diff we track the location where the macro argument is used and
(as it was before) the modifications originating from all the locations
except the first one are rejected, but multiple changes are allowed.
Test plan: make check-all
Differential revision: https://reviews.llvm.org/D33976
Added:
cfe/trunk/test/FixIt/fixit-format-darwin.m
Modified:
cfe/trunk/include/clang/Edit/EditedSource.h
cfe/trunk/lib/Edit/EditedSource.cpp
Modified: cfe/trunk/include/clang/Edit/EditedSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Edit/EditedSource.h?rev=305018&r1=305017&r2=305018&view=diff
==============================================================================
--- cfe/trunk/include/clang/Edit/EditedSource.h (original)
+++ cfe/trunk/include/clang/Edit/EditedSource.h Thu Jun 8 16:44:45 2017
@@ -41,9 +41,11 @@ class EditedSource {
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 @@ private:
FileEditsTy::iterator getActionForOffset(FileOffset Offs);
void deconstructMacroArgLoc(SourceLocation Loc,
SourceLocation &ExpansionLoc,
- IdentifierInfo *&II);
+ MacroArgUse &ArgUse);
void startingCommit();
void finishedCommit();
Modified: cfe/trunk/lib/Edit/EditedSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Edit/EditedSource.cpp?rev=305018&r1=305017&r2=305018&view=diff
==============================================================================
--- cfe/trunk/lib/Edit/EditedSource.cpp (original)
+++ cfe/trunk/lib/Edit/EditedSource.cpp Thu Jun 8 16:44:45 2017
@@ -25,17 +25,16 @@ void EditsReceiver::remove(CharSourceRan
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() {}
@@ -43,12 +42,11 @@ 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 @@ bool EditedSource::canInsertInOffset(Sou
}
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 @@ bool EditedSource::commitInsert(SourceLo
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];
Added: cfe/trunk/test/FixIt/fixit-format-darwin.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-format-darwin.m?rev=305018&view=auto
==============================================================================
--- cfe/trunk/test/FixIt/fixit-format-darwin.m (added)
+++ cfe/trunk/test/FixIt/fixit-format-darwin.m Thu Jun 8 16:44:45 2017
@@ -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());
+}
More information about the cfe-commits
mailing list