From 5964df144c21c548b9963f2ca35e0fa852b2f6f7 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis <akyrtzi@gmail.com> Date: Thu, 20 Dec 2012 21:05:53 +0000 Subject: [PATCH] Use some heuristics so that when a fixit removes a source range, we try to also remove a trailing space if possible. For example, removing '__bridge' from: i = (__bridge I*)p; should result in: i = (I*)p; not: i = ( I*)p; rdar://11314821 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170764 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Edit/EditedSource.cpp | 75 ++++++++++++++++++++++++++++-- test/Analysis/uninit-sometimes.cpp | 4 +- test/FixIt/bridge-in-non-arc.m | 12 +++++ 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 test/FixIt/bridge-in-non-arc.m diff --git a/lib/Edit/EditedSource.cpp b/lib/Edit/EditedSource.cpp index fe93b5f78c0..139a01fea8e 100644 --- a/lib/Edit/EditedSource.cpp +++ b/lib/Edit/EditedSource.cpp @@ -239,13 +239,82 @@ bool EditedSource::commit(const Commit &commit) { return true; } +static inline bool isIdentifierChar(char c, const LangOptions &LangOpts) { + return std::isalnum(c) || c == '_' || (c == '$' && LangOpts.DollarIdents); +} + +// \brief Returns true if it is ok to make the two given characters adjacent. +static bool canBeJoined(char left, char right, const LangOptions &LangOpts) { + // FIXME: Should use the Lexer to make sure we don't allow stuff like + // making two '<' adjacent. + return !(isIdentifierChar(left, LangOpts) && + isIdentifierChar(right, LangOpts)); +} + +/// \brief Returns true if it is ok to eliminate the trailing whitespace between +/// the given characters. +static bool canRemoveWhitespace(char left, char beforeWSpace, char right, + const LangOptions &LangOpts) { + if (!canBeJoined(left, right, LangOpts)) + return false; + if (std::isspace(left) || std::isspace(right)) + return true; + if (canBeJoined(beforeWSpace, right, LangOpts)) + return false; // the whitespace was intentional, keep it. + return true; +} + +/// \brief Check the range that we are going to remove and: +/// -Remove any trailing whitespace if possible. +/// -Insert a space if removing the range is going to mess up the source tokens. +static void adjustRemoval(const SourceManager &SM, const LangOptions &LangOpts, + SourceLocation Loc, FileOffset offs, + unsigned &len, StringRef &text) { + assert(len && text.empty()); + SourceLocation BeginTokLoc = Lexer::GetBeginningOfToken(Loc, SM, LangOpts); + if (BeginTokLoc != Loc) + return; // the range is not at the beginning of a token, keep the range. + + bool Invalid = false; + StringRef buffer = SM.getBufferData(offs.getFID(), &Invalid); + if (Invalid) + return; + + unsigned begin = offs.getOffset(); + unsigned end = begin + len; + + // FIXME: Remove newline. + + if (begin == 0) { + if (buffer[end] == ' ') + ++len; + return; + } + + if (buffer[end] == ' ') { + if (canRemoveWhitespace(/*left=*/buffer[begin-1], + /*beforeWSpace=*/buffer[end-1], + /*right=*/buffer[end+1], + LangOpts)) + ++len; + return; + } + + if (!canBeJoined(buffer[begin-1], buffer[end], LangOpts)) + text = " "; +} + static void applyRewrite(EditsReceiver &receiver, StringRef text, FileOffset offs, unsigned len, - const SourceManager &SM) { + const SourceManager &SM, const LangOptions &LangOpts) { assert(!offs.getFID().isInvalid()); SourceLocation Loc = SM.getLocForStartOfFile(offs.getFID()); Loc = Loc.getLocWithOffset(offs.getOffset()); assert(Loc.isFileID()); + + if (text.empty()) + adjustRemoval(SM, LangOpts, Loc, offs, len, text); + CharSourceRange range = CharSourceRange::getCharRange(Loc, Loc.getLocWithOffset(len)); @@ -288,14 +357,14 @@ void EditedSource::applyRewrites(EditsReceiver &receiver) { continue; } - applyRewrite(receiver, StrVec.str(), CurOffs, CurLen, SourceMgr); + applyRewrite(receiver, StrVec.str(), CurOffs, CurLen, SourceMgr, LangOpts); CurOffs = offs; StrVec = act.Text; CurLen = act.RemoveLen; CurEnd = CurOffs.getWithOffset(CurLen); } - applyRewrite(receiver, StrVec.str(), CurOffs, CurLen, SourceMgr); + applyRewrite(receiver, StrVec.str(), CurOffs, CurLen, SourceMgr, LangOpts); } void EditedSource::clearRewrites() { diff --git a/test/Analysis/uninit-sometimes.cpp b/test/Analysis/uninit-sometimes.cpp index 7825e873461..015b675d9b8 100644 --- a/test/Analysis/uninit-sometimes.cpp +++ b/test/Analysis/uninit-sometimes.cpp @@ -192,7 +192,7 @@ int test_logical_and_false(int k) { return x; // expected-note {{uninitialized use}} } -// CHECK: fix-it:"{{.*}}":{189:3-191:9}:"" +// CHECK: fix-it:"{{.*}}":{189:3-191:10}:"" // CHECK: fix-it:"{{.*}}":{188:8-188:8}:" = 0" @@ -232,7 +232,7 @@ int test_logical_or_true(int k) { return x; // expected-note {{uninitialized use}} } -// CHECK: fix-it:"{{.*}}":{229:3-231:9}:"" +// CHECK: fix-it:"{{.*}}":{229:3-231:10}:"" // CHECK: fix-it:"{{.*}}":{228:8-228:8}:" = 0" diff --git a/test/FixIt/bridge-in-non-arc.m b/test/FixIt/bridge-in-non-arc.m new file mode 100644 index 00000000000..948fa8ebcc1 --- /dev/null +++ b/test/FixIt/bridge-in-non-arc.m @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +@interface I +@end + +void foo(void *p) { + I *i = (__bridge I*)p; + I *i2 = (__bridge/*cake*/I*)p; +} + +// CHECK: {7:11-7:20}:"" +// CHECK: {8:12-8:20}:"" -- GitLab