SureYeaah created this revision.
SureYeaah added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, ilya-biryukov.
Herald added a project: clang.
- Removed assumption that begin and end need to be in same file id.
- Fixes the crash at https://github.com/clangd/clangd/issues/113
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65754
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ 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<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 @@
CheckRange("c");
CheckRange("d");
CheckRange("e");
+ CheckRange("f");
}
} // namespace
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -295,37 +295,63 @@
return SourceRange(std::min(R1.getBegin(), R2.getBegin()),
E1 < E2 ? R2.getEnd() : R1.getEnd());
}
-
-// Returns the tokenFileRange for a given Location as a Token Range
+// If Loc is a macro arg expansion, return the immediate spelling loc.
+// Otherwise
+// Assumes Loc is in a macro file ID
+static SourceRange getExpansionOrSpellingRange(SourceLocation Loc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (SM.isMacroArgExpansion(Loc))
+ return SM.getImmediateSpellingLoc(Loc);
+ return toTokenRange(SM.getImmediateExpansionRange(Loc), SM, LangOpts);
+}
+
+// Returns the tokenFileRange for a given Range 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,
- const SourceManager &SM,
+static SourceRange getTokenFileRange(SourceRange Rng, const SourceManager &SM,
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()));
- } else {
- SourceRange ExpansionRangeForBegin = toTokenRange(
- SM.getImmediateExpansionRange(FileRange.getBegin()), SM, LangOpts);
- SourceRange ExpansionRangeForEnd = toTokenRange(
- SM.getImmediateExpansionRange(FileRange.getEnd()), SM, LangOpts);
- FileRange = unionTokenRange(ExpansionRangeForBegin, ExpansionRangeForEnd,
- SM, LangOpts);
- }
+ // Map from FileID hash to SourceRange
+ std::map<unsigned, SourceRange> RngMap;
+ // Adds a Location to the RngMap
+ auto AddLocToMap = [&RngMap, &SM, &LangOpts](SourceLocation Loc) {
+ unsigned FileIDHash = SM.getFileID(Loc).getHashValue();
+ auto It = RngMap.find(FileIDHash);
+ if (It == RngMap.end())
+ RngMap[FileIDHash] = Loc;
+ else
+ It->second = unionTokenRange(It->second, Loc, SM, LangOpts);
+ };
+ auto AddRngToMap = [&](SourceRange SR) {
+ AddLocToMap(SR.getBegin());
+ AddLocToMap(SR.getEnd());
+ };
+ // Initialize RngMap
+ AddRngToMap(Rng);
+ // Iterate until only one range is left which is not a macroID
+ for (auto It = RngMap.rbegin();
+ !RngMap.empty() && It->second.getBegin().isMacroID();
+ It = RngMap.rbegin()) {
+ // Get SourceRange with maximum FileID and remove from RngMap
+ Rng = It->second;
+ RngMap.erase(It->first);
+ // Add Immediate Expansion/Spelling Range to RngMap
+ AddRngToMap(getExpansionOrSpellingRange(Rng.getBegin(), SM, LangOpts));
+ AddRngToMap(getExpansionOrSpellingRange(Rng.getEnd(), SM, LangOpts));
}
- return FileRange;
+ assert(RngMap.size() == 1 && "There should be only one final range.");
+ SourceRange TokenFileRange = RngMap.begin()->second;
+ assert(isInsideMainFile(TokenFileRange.getBegin(), SM) &&
+ isInsideMainFile(TokenFileRange.getBegin(), SM) &&
+ "Final range should be in main file.");
+ return TokenFileRange;
}
bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM) {
@@ -335,15 +361,10 @@
llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &SM,
const LangOptions &LangOpts,
SourceRange R) {
- SourceRange R1 = getTokenFileRange(R.getBegin(), SM, LangOpts);
- if (!isValidFileRange(SM, R1))
- return llvm::None;
-
- SourceRange R2 = getTokenFileRange(R.getEnd(), SM, LangOpts);
- if (!isValidFileRange(SM, R2))
+ SourceRange Result = getTokenFileRange(R, SM, LangOpts);
+ if (!isValidFileRange(SM, Result))
return llvm::None;
- SourceRange Result = unionTokenRange(R1, R2, SM, LangOpts);
unsigned TokLen = getTokenLengthAtLoc(Result.getEnd(), SM, LangOpts);
// Convert from closed token range to half-open (char) range
Result.setEnd(Result.getEnd().getLocWithOffset(TokLen));
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -369,7 +369,8 @@
// }
// Selecting "++x" or "x" will do the right thing.
auto Range = toHalfOpenFileRange(SM, LangOpts, S);
- assert(Range && "We should be able to get the File Range");
+ if(!Range)
+ return SelectionTree::Unselected;
dlog("{1}claimRange: {0}", Range->printToString(SM), indent());
auto B = SM.getDecomposedLoc(Range->getBegin());
auto E = SM.getDecomposedLoc(Range->getEnd());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
