[clang-tools-extra] r368058 - Fixed toHalfOpenFileRange assertion fail
Shaurya Gupta via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 6 10:01:13 PDT 2019
Author: sureyeaah
Date: Tue Aug 6 10:01:12 2019
New Revision: 368058
URL: http://llvm.org/viewvc/llvm-project?rev=368058&view=rev
Log:
Fixed toHalfOpenFileRange assertion fail
Summary:
- Added new function that gets Expansion range with both ends in the same file.
- Fixes the crash at https://github.com/clangd/clangd/issues/113
Subscribers: ilya-biryukov, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65754
Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=368058&r1=368057&r2=368058&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Aug 6 10:01:12 2019
@@ -296,14 +296,52 @@ static SourceRange unionTokenRange(Sourc
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
+getExpansionTokenRangeInSameFile(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;
+ // Record the stack of expansion locations for the beginning, keyed by FileID.
+ llvm::DenseMap<FileID, SourceLocation> BeginExpansions;
+ for (SourceLocation Begin = ExpansionRange.getBegin(); Begin.isValid();
+ Begin = Begin.isFileID()
+ ? SourceLocation()
+ : SM.getImmediateExpansionRange(Begin).getBegin()) {
+ BeginExpansions[SM.getFileID(Begin)] = Begin;
+ }
+ // Move up the stack of expansion locations for the end until we find the
+ // location in BeginExpansions with that has the same file id.
+ for (SourceLocation End = ExpansionRange.getEnd(); End.isValid();
+ End = End.isFileID() ? SourceLocation()
+ : toTokenRange(SM.getImmediateExpansionRange(End),
+ SM, LangOpts)
+ .getEnd()) {
+ auto It = BeginExpansions.find(SM.getFileID(End));
+ if (It != BeginExpansions.end())
+ return {It->second, End};
+ }
+ 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 +349,19 @@ static SourceRange getTokenFileRange(Sou
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));
} else {
- SourceRange ExpansionRangeForBegin = toTokenRange(
- SM.getImmediateExpansionRange(FileRange.getBegin()), SM, LangOpts);
- SourceRange ExpansionRangeForEnd = toTokenRange(
- SM.getImmediateExpansionRange(FileRange.getEnd()), SM, LangOpts);
+ SourceRange ExpansionRangeForBegin =
+ getExpansionTokenRangeInSameFile(FileRange.getBegin(), SM, LangOpts);
+ SourceRange ExpansionRangeForEnd =
+ getExpansionTokenRangeInSameFile(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);
}
Modified: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp?rev=368058&r1=368057&r2=368058&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp Tue Aug 6 10:01:12 2019
@@ -460,15 +460,22 @@ TEST(SourceCodeTests, HalfOpenFileRange)
#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<typename T>
class P {};
- void f() {
+
+ int main() {
$a[[P<P<P<P<P<int>>>>> 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 @@ TEST(SourceCodeTests, HalfOpenFileRange)
CheckRange("c");
CheckRange("d");
CheckRange("e");
+ CheckRange("f");
}
} // namespace
More information about the cfe-commits
mailing list