diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -296,14 +296,57 @@ E1 < E2 ? R2.getEnd() : R1.getEnd()); } -// Returns the tokenFileRange for a given Location as a Token Range +// Check if two locations have the same file id. +static bool inSameFile(SourceLocation Loc1, SourceLocation Loc2, + const SourceManager &SM) { + return SM.getFileID(Loc1) == SM.getFileID(Loc2); +} + +// Find an expansion range (not necessarily immediate) the ends of which are in +// the same file id. +static SourceRange getExpansionRangeInSameFiles(SourceLocation Loc, + const SourceManager &SM, + const LangOptions &LangOpts) { + SourceRange ExpansionRange = + toTokenRange(SM.getImmediateExpansionRange(Loc), SM, LangOpts); + // Fast path for most common cases. + if (inSameFile(ExpansionRange.getBegin(), ExpansionRange.getEnd(), SM)) + return ExpansionRange; + // Expand begin of Expansion range till the top and map with file id hash. + llvm::DenseMap BeginExpansions; + SourceRange BeginExpansion = ExpansionRange.getBegin(); + while (1) { + BeginExpansions[SM.getFileID(BeginExpansion.getBegin()).getHashValue()] = + BeginExpansion; + if (BeginExpansion.getBegin().isFileID()) + break; + BeginExpansion = toTokenRange( + SM.getImmediateExpansionRange(BeginExpansion.getBegin()), SM, LangOpts); + } + // Expand the end of Expansion range till the top and find the first match in + // BeginExpansions. + SourceRange EndExpansion = ExpansionRange.getEnd(); + while (1) { + auto It = BeginExpansions.find( + SM.getFileID(EndExpansion.getEnd()).getHashValue()); + if (It != BeginExpansions.end()) + return unionTokenRange(It->second, EndExpansion, SM, LangOpts); + if (EndExpansion.getEnd().isFileID()) + break; + EndExpansion = toTokenRange( + SM.getImmediateExpansionRange(EndExpansion.getEnd()), SM, LangOpts); + } + llvm_unreachable( + "We should able to find a common ancestor in the expansion tree."); +} +// Returns the file range for a given Location as a Token Range // This is quite similar to getFileLoc in SourceManager as both use // getImmediateExpansionRange and getImmediateSpellingLoc (for macro IDs). // However: // - We want to maintain the full range information as we move from one file to // the next. getFileLoc only uses the BeginLoc of getImmediateExpansionRange. -// - We want to split '>>' tokens as the lexer parses the '>>' in template -// instantiations as a '>>' instead of a '>'. +// - We want to split '>>' tokens as the lexer parses the '>>' in nested +// template instantiations as a '>>' instead of two '>'s. // There is also getExpansionRange but it simply calls // getImmediateExpansionRange on the begin and ends separately which is wrong. static SourceRange getTokenFileRange(SourceLocation Loc, @@ -311,16 +354,20 @@ const LangOptions &LangOpts) { SourceRange FileRange = Loc; while (!FileRange.getBegin().isFileID()) { - assert(!FileRange.getEnd().isFileID() && - "Both Begin and End should be MacroIDs."); if (SM.isMacroArgExpansion(FileRange.getBegin())) { - FileRange.setBegin(SM.getImmediateSpellingLoc(FileRange.getBegin())); - FileRange.setEnd(SM.getImmediateSpellingLoc(FileRange.getEnd())); + FileRange = unionTokenRange( + SM.getImmediateSpellingLoc(FileRange.getBegin()), + SM.getImmediateSpellingLoc(FileRange.getEnd()), SM, LangOpts); + assert(inSameFile(FileRange.getBegin(), FileRange.getEnd(), SM) && + "Both ends should be in same file."); } else { - SourceRange ExpansionRangeForBegin = toTokenRange( - SM.getImmediateExpansionRange(FileRange.getBegin()), SM, LangOpts); - SourceRange ExpansionRangeForEnd = toTokenRange( - SM.getImmediateExpansionRange(FileRange.getEnd()), SM, LangOpts); + SourceRange ExpansionRangeForBegin = + getExpansionRangeInSameFiles(FileRange.getBegin(), SM, LangOpts); + SourceRange ExpansionRangeForEnd = + getExpansionRangeInSameFiles(FileRange.getEnd(), SM, LangOpts); + assert(inSameFile(ExpansionRangeForBegin.getBegin(), + ExpansionRangeForEnd.getBegin(), SM) && + "Both Expansion ranges should be in same file."); FileRange = unionTokenRange(ExpansionRangeForBegin, ExpansionRangeForEnd, SM, LangOpts); } diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -460,15 +460,22 @@ #define FOO(X, Y) int Y = ++X #define BAR(X) X + 1 #define ECHO(X) X + + #define BUZZ BAZZ(ADD) + #define BAZZ(m) m(1) + #define ADD(a) int f = a + 1; template class P {}; - void f() { + + int main() { $a[[P>>>> a]]; $b[[int b = 1]]; $c[[FOO(b, c)]]; $d[[FOO(BAR(BAR(b)), d)]]; // FIXME: We might want to select everything inside the outer ECHO. ECHO(ECHO($e[[int) ECHO(e]])); + // Shouldn't crash. + $f[[BUZZ]]; } )cpp"); @@ -495,6 +502,7 @@ CheckRange("c"); CheckRange("d"); CheckRange("e"); + CheckRange("f"); } } // namespace