From: Argyrios Kyrtzidis Date: Fri, 11 Sep 2015 20:09:11 +0000 (+0000) Subject: [Edit] Fix issue with tracking what macro argument inputs have been edited. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=aa98950ec9ff900bdd09ed21cced65670d5b1e75;p=clang [Edit] Fix issue with tracking what macro argument inputs have been edited. This was not working correctly, leading to erroneously rejecting valid edits. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@247462 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Edit/EditedSource.h b/include/clang/Edit/EditedSource.h index 8bc27e73be..b6ec8b8f06 100644 --- a/include/clang/Edit/EditedSource.h +++ b/include/clang/Edit/EditedSource.h @@ -10,9 +10,11 @@ #ifndef LLVM_CLANG_EDIT_EDITEDSOURCE_H #define LLVM_CLANG_EDIT_EDITEDSOURCE_H +#include "clang/Basic/IdentifierTable.h" #include "clang/Edit/FileOffset.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/TinyPtrVector.h" #include "llvm/Support/Allocator.h" #include @@ -39,14 +41,18 @@ class EditedSource { typedef std::map FileEditsTy; FileEditsTy FileEdits; - llvm::DenseMap ExpansionToArgMap; + llvm::DenseMap> + ExpansionToArgMap; + SmallVector, 2> + CurrCommitMacroArgExps; + IdentifierTable IdentTable; llvm::BumpPtrAllocator StrAlloc; public: EditedSource(const SourceManager &SM, const LangOptions &LangOpts, const PPConditionalDirectiveRecord *PPRec = nullptr) - : SourceMgr(SM), LangOpts(LangOpts), PPRec(PPRec), + : SourceMgr(SM), LangOpts(LangOpts), PPRec(PPRec), IdentTable(LangOpts), StrAlloc() { } const SourceManager &getSourceManager() const { return SourceMgr; } @@ -76,6 +82,12 @@ private: StringRef getSourceText(FileOffset BeginOffs, FileOffset EndOffs, bool &Invalid); FileEditsTy::iterator getActionForOffset(FileOffset Offs); + void deconstructMacroArgLoc(SourceLocation Loc, + SourceLocation &ExpansionLoc, + IdentifierInfo *&II); + + void startingCommit(); + void finishedCommit(); }; } diff --git a/lib/Edit/EditedSource.cpp b/lib/Edit/EditedSource.cpp index e557de9241..54659f6e4e 100644 --- a/lib/Edit/EditedSource.cpp +++ b/lib/Edit/EditedSource.cpp @@ -23,6 +23,36 @@ void EditsReceiver::remove(CharSourceRange range) { replace(range, StringRef()); } +void EditedSource::deconstructMacroArgLoc(SourceLocation Loc, + SourceLocation &ExpansionLoc, + IdentifierInfo *&II) { + 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); + } +} + +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); + } + } + CurrCommitMacroArgExps.clear(); +} + StringRef EditedSource::copyString(const Twine &twine) { SmallString<128> Data; return copyString(twine.toStringRef(Data)); @@ -36,15 +66,27 @@ bool EditedSource::canInsertInOffset(SourceLocation OrigLoc, FileOffset Offs) { } if (SourceMgr.isMacroArgExpansion(OrigLoc)) { - SourceLocation - DefArgLoc = SourceMgr.getImmediateExpansionRange(OrigLoc).first; - SourceLocation - ExpLoc = SourceMgr.getImmediateExpansionRange(DefArgLoc).first; - llvm::DenseMap::iterator - I = ExpansionToArgMap.find(ExpLoc.getRawEncoding()); - if (I != ExpansionToArgMap.end() && I->second != DefArgLoc) - return false; // Trying to write in a macro argument input that has - // already been written for another argument of the same macro. + IdentifierInfo *II; + SourceLocation ExpLoc; + deconstructMacroArgLoc(OrigLoc, ExpLoc, II); + auto I = ExpansionToArgMap.find(ExpLoc.getRawEncoding()); + if (I != ExpansionToArgMap.end() && + std::find(I->second.begin(), I->second.end(), II) != 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: + // + // \code + // #define MAC(x) ((x)+(x)) + // MAC(a) + // \endcode + // + // A commit modified the macro argument 'a' due to the first '(x)' + // expansion inside the macro definition, and a subsequent commit tried + // to modify 'a' again for the second '(x)' expansion. The edits of the + // second commit will be rejected. + return false; + } } return true; @@ -59,11 +101,11 @@ bool EditedSource::commitInsert(SourceLocation OrigLoc, return true; if (SourceMgr.isMacroArgExpansion(OrigLoc)) { - SourceLocation - DefArgLoc = SourceMgr.getImmediateExpansionRange(OrigLoc).first; - SourceLocation - ExpLoc = SourceMgr.getImmediateExpansionRange(DefArgLoc).first; - ExpansionToArgMap[ExpLoc.getRawEncoding()] = DefArgLoc; + IdentifierInfo *II; + SourceLocation ExpLoc; + deconstructMacroArgLoc(OrigLoc, ExpLoc, II); + if (II) + CurrCommitMacroArgExps.emplace_back(ExpLoc, II); } FileEdit &FA = FileEdits[Offs]; @@ -219,6 +261,16 @@ bool EditedSource::commit(const Commit &commit) { if (!commit.isCommitable()) return false; + struct CommitRAII { + EditedSource &Editor; + CommitRAII(EditedSource &Editor) : Editor(Editor) { + Editor.startingCommit(); + } + ~CommitRAII() { + Editor.finishedCommit(); + } + } CommitRAII(*this); + for (edit::Commit::edit_iterator I = commit.edit_begin(), E = commit.edit_end(); I != E; ++I) { const edit::Commit::Edit &edit = *I; diff --git a/test/ARCMT/objcmt-subscripting-literals.m b/test/ARCMT/objcmt-subscripting-literals.m index 014c109299..0974c3b8bb 100644 --- a/test/ARCMT/objcmt-subscripting-literals.m +++ b/test/ARCMT/objcmt-subscripting-literals.m @@ -88,6 +88,7 @@ typedef const struct __CFString * CFStringRef; #define M(x) (x) #define PAIR(x) @#x, [NSNumber numberWithInt:(x)] #define TWO(x) ((x), (x)) +#define TWO_SEP(x,y) ((x), (y)) @interface I { NSArray *ivarArr; @@ -118,6 +119,7 @@ typedef const struct __CFString * CFStringRef; id o = [arr objectAtIndex:2]; o = [dict objectForKey:@"key"]; o = TWO([dict objectForKey:@"key"]); + o = TWO_SEP([dict objectForKey:@"key"], [arr objectAtIndex:2]); o = [NSDictionary dictionaryWithObject:[NSDictionary dictionary] forKey:@"key"]; NSMutableArray *marr = 0; NSMutableDictionary *mdict = 0; diff --git a/test/ARCMT/objcmt-subscripting-literals.m.result b/test/ARCMT/objcmt-subscripting-literals.m.result index e9ff8df34d..ed7879bb13 100644 --- a/test/ARCMT/objcmt-subscripting-literals.m.result +++ b/test/ARCMT/objcmt-subscripting-literals.m.result @@ -88,6 +88,7 @@ typedef const struct __CFString * CFStringRef; #define M(x) (x) #define PAIR(x) @#x, [NSNumber numberWithInt:(x)] #define TWO(x) ((x), (x)) +#define TWO_SEP(x,y) ((x), (y)) @interface I { NSArray *ivarArr; @@ -118,6 +119,7 @@ typedef const struct __CFString * CFStringRef; id o = arr[2]; o = dict[@"key"]; o = TWO(dict[@"key"]); + o = TWO_SEP(dict[@"key"], arr[2]); o = @{@"key": @{}}; NSMutableArray *marr = 0; NSMutableDictionary *mdict = 0;