[clang-tools-extra] b60896f - [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 13 07:57:16 PST 2019
Author: Sam McCall
Date: 2019-12-13T16:57:03+01:00
New Revision: b60896fad926754f715acc5d771555aaaa577e0f
URL: https://github.com/llvm/llvm-project/commit/b60896fad926754f715acc5d771555aaaa577e0f
DIFF: https://github.com/llvm/llvm-project/commit/b60896fad926754f715acc5d771555aaaa577e0f.diff
LOG: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.
Summary:
The problem:
LSP specifies that Positions are between characters. Therefore when a position
(or an empty range) is used to target elements of the source code, there is an
ambiguity - should we look left or right of the cursor?
Until now, SelectionTree resolved this to the right except in trivial cases
(where there's whitespace, semicolon, or eof on the right).
This meant that it's unable to e.g. out-line `int foo^()` today.
Complicating this, LSP notwithstanding the cursor is *on* a character in many
editors (mostly terminal-based). In these cases there's no ambiguity - we must
"look right" - but there's also no way to tell in LSP.
(Several features currently resolve this by using getBeginningOfIdentifier,
which tries to rewind and supports end-of-identifier. But this relies on
raw lexing and is limited and buggy).
Precedent: well - most other languages aren't so full of densely packed symbols
that we might want to target. Bias-towards-identifier works well enough.
MS C++ for vscode seems to mostly use bias-toward-identifier too.
The problem with this solution is it doesn't provide any way to target some
things such as the constructor call in Foo^(bar());
Presented solution:
When an ambiguous selection is found, we generate *both* possible selection
trees. We try to run the feature on the rightward tree first, and then on the
leftward tree if it fails.
This is basically do-what-I-mean, the main downside is the need to do this on
a feature-by-feature basis (because each feature knows what "fail" means).
The most complicated instance of this is Tweaks, where the preferred selection
may vary tweak-by-tweak.
Wrinkles:
While production behavior is pretty consistent, this introduces some
inconsistency in testing, depending whether the interface we're testing is
inside or outside the "retry" wrapper.
In particular, for many features like Hover, the unit tests will show production
behavior, while for Tweaks the harness would have to run the loop itself if
we want this.
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D71345
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/Selection.h
clang-tools-extra/clangd/SemanticSelection.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index a858680d4067..7934408ff141 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -374,7 +374,8 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
WorkScheduler.runWithAST("Rename", File, std::move(Action));
}
-static llvm::Expected<Tweak::Selection>
+// May generate several candidate selections, due to SelectionTree ambiguity.
+static llvm::Expected<std::vector<Tweak::Selection>>
tweakSelection(const Range &Sel, const InputsAndAST &AST) {
auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
if (!Begin)
@@ -382,7 +383,15 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
if (!End)
return End.takeError();
- return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
+ std::vector<Tweak::Selection> Result;
+ SelectionTree::createEach(AST.AST.getASTContext(), AST.AST.getTokens(),
+ *Begin, *End, [&](SelectionTree T) {
+ Result.emplace_back(AST.Inputs.Index, AST.AST,
+ *Begin, *End, std::move(T));
+ return false;
+ });
+ assert(!Result.empty() && "Expected at least one SelectionTree");
+ return Result;
}
void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
@@ -391,12 +400,21 @@ void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
this](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selection = tweakSelection(Sel, *InpAST);
- if (!Selection)
- return CB(Selection.takeError());
+ auto Selections = tweakSelection(Sel, *InpAST);
+ if (!Selections)
+ return CB(Selections.takeError());
std::vector<TweakRef> Res;
- for (auto &T : prepareTweaks(*Selection, TweakFilter))
- Res.push_back({T->id(), T->title(), T->intent()});
+ // Don't allow a tweak to fire more than once across ambiguous selections.
+ llvm::DenseSet<llvm::StringRef> PreparedTweaks;
+ auto Filter = [&](const Tweak &T) {
+ return TweakFilter(T) && !PreparedTweaks.count(T.id());
+ };
+ for (const auto &Sel : *Selections) {
+ for (auto &T : prepareTweaks(Sel, Filter)) {
+ Res.push_back({T->id(), T->title(), T->intent()});
+ PreparedTweaks.insert(T->id());
+ }
+ }
CB(std::move(Res));
};
@@ -411,21 +429,30 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selection = tweakSelection(Sel, *InpAST);
- if (!Selection)
- return CB(Selection.takeError());
- auto A = prepareTweak(TweakID, *Selection);
- if (!A)
- return CB(A.takeError());
- auto Effect = (*A)->apply(*Selection);
- if (!Effect)
- return CB(Effect.takeError());
- for (auto &It : Effect->ApplyEdits) {
- Edit &E = It.second;
- format::FormatStyle Style =
- getFormatStyleForFile(File, E.InitialCode, FS.get());
- if (llvm::Error Err = reformatEdit(E, Style))
- elog("Failed to format {0}: {1}", It.first(), std::move(Err));
+ auto Selections = tweakSelection(Sel, *InpAST);
+ if (!Selections)
+ return CB(Selections.takeError());
+ llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;
+ // Try each selection, take the first one that prepare()s.
+ // If they all fail, Effect will hold get the last error.
+ for (const auto &Selection : *Selections) {
+ auto T = prepareTweak(TweakID, Selection);
+ if (T) {
+ Effect = (*T)->apply(Selection);
+ break;
+ }
+ Effect = T.takeError();
+ }
+ assert(Effect.hasValue() && "Expected at least one selection");
+ if (*Effect) {
+ // Tweaks don't apply clang-format, do that centrally here.
+ for (auto &It : (*Effect)->ApplyEdits) {
+ Edit &E = It.second;
+ format::FormatStyle Style =
+ getFormatStyleForFile(File, E.InitialCode, FS.get());
+ if (llvm::Error Err = reformatEdit(E, Style))
+ elog("Failed to format {0}: {1}", It.first(), std::move(Err));
+ }
}
return CB(std::move(*Effect));
};
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index f55a9c3d1f67..904ae3e49d13 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -410,9 +410,12 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
llvm::consumeError(Offset.takeError());
return llvm::None;
}
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
+ // Editors send the position on the left of the hovered character.
+ // So our selection tree should be biased right. (Tested with VSCode).
+ SelectionTree ST = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), *Offset, *Offset);
std::vector<const Decl *> Result;
- if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+ if (const SelectionTree::Node *N = ST.commonAncestor()) {
DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
auto Decls = targetDecl(N->ASTNode, Rel);
if (!Decls.empty()) {
diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index ffa48f3a57d9..4dae2f735397 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -142,6 +142,11 @@ void update(SelectionTree::Selection &Result, SelectionTree::Selection New) {
Result = SelectionTree::Partial;
}
+// As well as comments, don't count semicolons as real tokens.
+// They're not properly claimed as expr-statement is missing from the AST.
+bool shouldIgnore(const syntax::Token &Tok) {
+ return Tok.kind() == tok::comment || Tok.kind() == tok::semi;
+}
// SelectionTester can determine whether a range of tokens from the PP-expanded
// stream (corresponding to an AST node) is considered selected.
@@ -172,9 +177,7 @@ class SelectionTester {
});
// Precompute selectedness and offset for selected spelled tokens.
for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) {
- // As well as comments, don't count semicolons as real tokens.
- // They're not properly claimed as expr-statement is missing from the AST.
- if (T->kind() == tok::comment || T->kind() == tok::semi)
+ if (shouldIgnore(*T))
continue;
SpelledTokens.emplace_back();
Tok &S = SpelledTokens.back();
@@ -650,24 +653,49 @@ std::string SelectionTree::Node::kind() const {
return std::move(OS.str());
}
-// Decide which selection emulates a "point" query in between characters.
-static std::pair<unsigned, unsigned> pointBounds(unsigned Offset, FileID FID,
- ASTContext &AST) {
- StringRef Buf = AST.getSourceManager().getBufferData(FID);
- // Edge-cases where the choice is forced.
- if (Buf.size() == 0)
- return {0, 0};
- if (Offset == 0)
- return {0, 1};
- if (Offset == Buf.size())
- return {Offset - 1, Offset};
- // We could choose either this byte or the previous. Usually we prefer the
- // character on the right of the cursor (or under a block cursor).
- // But if that's whitespace/semicolon, we likely want the token on the left.
- auto IsIgnoredChar = [](char C) { return isWhitespace(C) || C == ';'; };
- if (IsIgnoredChar(Buf[Offset]) && !IsIgnoredChar(Buf[Offset - 1]))
- return {Offset - 1, Offset};
- return {Offset, Offset + 1};
+// Decide which selections emulate a "point" query in between characters.
+// If it's ambiguous (the neighboring characters are selectable tokens), returns
+// both possibilities in preference order.
+// Always returns at least one range - if no tokens touched, and empty range.
+static llvm::SmallVector<std::pair<unsigned, unsigned>, 2>
+pointBounds(unsigned Offset, const syntax::TokenBuffer &Tokens) {
+ const auto &SM = Tokens.sourceManager();
+ SourceLocation Loc = SM.getComposedLoc(SM.getMainFileID(), Offset);
+ llvm::SmallVector<std::pair<unsigned, unsigned>, 2> Result;
+ // Prefer right token over left.
+ for (const syntax::Token &Tok :
+ llvm::reverse(spelledTokensTouching(Loc, Tokens))) {
+ if (shouldIgnore(Tok))
+ continue;
+ unsigned Offset = Tokens.sourceManager().getFileOffset(Tok.location());
+ Result.emplace_back(Offset, Offset + Tok.length());
+ }
+ if (Result.empty())
+ Result.emplace_back(Offset, Offset);
+ return Result;
+}
+
+bool SelectionTree::createEach(ASTContext &AST,
+ const syntax::TokenBuffer &Tokens,
+ unsigned Begin, unsigned End,
+ llvm::function_ref<bool(SelectionTree)> Func) {
+ if (Begin != End)
+ return Func(SelectionTree(AST, Tokens, Begin, End));
+ for (std::pair<unsigned, unsigned> Bounds : pointBounds(Begin, Tokens))
+ if (Func(SelectionTree(AST, Tokens, Bounds.first, Bounds.second)))
+ return true;
+ return false;
+}
+
+SelectionTree SelectionTree::createRight(ASTContext &AST,
+ const syntax::TokenBuffer &Tokens,
+ unsigned int Begin, unsigned int End) {
+ llvm::Optional<SelectionTree> Result;
+ createEach(AST, Tokens, Begin, End, [&](SelectionTree T) {
+ Result = std::move(T);
+ return true;
+ });
+ return std::move(*Result);
}
SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
@@ -677,8 +705,6 @@ SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
// but that's all clangd has needed so far.
const SourceManager &SM = AST.getSourceManager();
FileID FID = SM.getMainFileID();
- if (Begin == End)
- std::tie(Begin, End) = pointBounds(Begin, FID, AST);
PrintPolicy.TerseOutput = true;
PrintPolicy.IncludeNewlines = false;
@@ -690,10 +716,6 @@ SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
dlog("Built selection tree\n{0}", *this);
}
-SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Offset)
- : SelectionTree(AST, Tokens, Offset, Offset) {}
-
const Node *SelectionTree::commonAncestor() const {
const Node *Ancestor = Root;
while (Ancestor->Children.size() == 1 && !Ancestor->Selected)
diff --git a/clang-tools-extra/clangd/Selection.h b/clang-tools-extra/clangd/Selection.h
index a7050c49be6b..36196f812db9 100644
--- a/clang-tools-extra/clangd/Selection.h
+++ b/clang-tools-extra/clangd/Selection.h
@@ -29,6 +29,14 @@
// - we determine which low-level nodes are partly or completely covered
// by the selection.
// - we expose a tree of the selected nodes and their lexical parents.
+//
+// Sadly LSP specifies locations as being between characters, and this causes
+// some ambiguities we cannot cleanly resolve:
+// lhs+rhs // targeting '+' or 'lhs'?
+// ^ // in GUI editors, double-clicking 'lhs' yields this position!
+//
+// The best we can do in these cases is try both, which leads to the awkward
+// SelectionTree::createEach() API.
//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SELECTION_H
@@ -64,16 +72,32 @@ namespace clangd {
// point back into the AST it was constructed with.
class SelectionTree {
public:
- // Creates a selection tree at the given byte offset in the main file.
- // This is approximately equivalent to a range of one character.
- // (Usually, the character to the right of Offset, sometimes to the left).
- SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Offset);
- // Creates a selection tree for the given range in the main file.
- // The range includes bytes [Start, End).
- // If Start == End, uses the same heuristics as SelectionTree(AST, Start).
- SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
- unsigned Start, unsigned End);
+ // Create selection trees for the given range, and pass them to Func.
+ //
+ // There may be multiple possible selection trees:
+ // - if the range is empty and borders two tokens, a tree for the right token
+ // and a tree for the left token will be yielded.
+ // - Func should return true on success (stop) and false on failure (continue)
+ //
+ // Always yields at least one tree. If no tokens are touched, it is empty.
+ static bool createEach(ASTContext &AST, const syntax::TokenBuffer &Tokens,
+ unsigned Begin, unsigned End,
+ llvm::function_ref<bool(SelectionTree)> Func);
+
+ // Create a selection tree for the given range.
+ //
+ // Where ambiguous (range is empty and borders two tokens), prefer the token
+ // on the right.
+ static SelectionTree createRight(ASTContext &AST,
+ const syntax::TokenBuffer &Tokens,
+ unsigned Begin, unsigned End);
+
+ // Copies are no good - contain pointers to other nodes.
+ SelectionTree(const SelectionTree &) = delete;
+ SelectionTree &operator=(const SelectionTree &) = delete;
+ // Moves are OK though - internal storage is pointer-stable when moved.
+ SelectionTree(SelectionTree &&) = default;
+ SelectionTree &operator=(SelectionTree &&) = default;
// Describes to what extent an AST node is covered by the selection.
enum Selection : unsigned char {
@@ -121,6 +145,11 @@ class SelectionTree {
const Node &root() const { return *Root; }
private:
+ // Creates a selection tree for the given range in the main file.
+ // The range includes bytes [Start, End).
+ SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
+ unsigned Start, unsigned End);
+
std::deque<Node> Nodes; // Stable-pointer storage.
const Node *Root;
clang::PrintingPolicy PrintPolicy;
diff --git a/clang-tools-extra/clangd/SemanticSelection.cpp b/clang-tools-extra/clangd/SemanticSelection.cpp
index cbbf31f1b05b..49fc7703db05 100644
--- a/clang-tools-extra/clangd/SemanticSelection.cpp
+++ b/clang-tools-extra/clangd/SemanticSelection.cpp
@@ -39,7 +39,8 @@ llvm::Expected<std::vector<Range>> getSemanticRanges(ParsedAST &AST,
}
// Get node under the cursor.
- SelectionTree ST(AST.getASTContext(), AST.getTokens(), *Offset);
+ SelectionTree ST = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), *Offset, *Offset);
for (const auto *Node = ST.commonAncestor(); Node != nullptr;
Node = Node->Parent) {
if (const Decl *D = Node->ASTNode.get<Decl>()) {
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index d9a54ad971c4..7b52164e90e9 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -131,15 +131,16 @@ SymbolLocation getPreferredLocation(const Location &ASTLoc,
std::vector<const Decl *> getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
DeclRelationSet Relations) {
- FileID FID;
- unsigned Offset;
- std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+ unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Pos).second;
std::vector<const Decl *> Result;
- if (const SelectionTree::Node *N = Selection.commonAncestor()) {
- auto Decls = targetDecl(N->ASTNode, Relations);
- Result.assign(Decls.begin(), Decls.end());
- }
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
+ Offset, [&](SelectionTree ST) {
+ if (const SelectionTree::Node *N =
+ ST.commonAncestor())
+ llvm::copy(targetDecl(N->ASTNode, Relations),
+ std::back_inserter(Result));
+ return !Result.empty();
+ });
return Result;
}
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 010f7e388b55..52041a558759 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -80,7 +80,8 @@ llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST,
unsigned Offset =
AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+ SelectionTree Selection = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), Offset, Offset);
const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
if (!SelectedNode)
return {};
diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 2dc091ed762a..dd4ac3a21651 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -46,10 +46,10 @@ void validateRegistry() {
} // namespace
Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
- unsigned RangeBegin, unsigned RangeEnd)
+ unsigned RangeBegin, unsigned RangeEnd,
+ SelectionTree ASTSelection)
: Index(Index), AST(AST), SelectionBegin(RangeBegin),
- SelectionEnd(RangeEnd),
- ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) {
+ SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
auto &SM = AST.getSourceManager();
Code = SM.getBufferData(SM.getMainFileID());
Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h
index 69ac4ad612e9..6e6ebd18cffc 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -48,7 +48,7 @@ class Tweak {
/// Input to prepare and apply tweaks.
struct Selection {
Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
- unsigned RangeEnd);
+ unsigned RangeEnd, SelectionTree ASTSelection);
/// The text of the active document.
llvm::StringRef Code;
/// The Index for handling codebase related queries.
diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index a81569485e89..02dbd36697f9 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -77,8 +77,8 @@ class TargetDeclTest : public ::testing::Test {
auto AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty()) << Code;
llvm::Annotations::Range R = A.range();
- SelectionTree Selection(AST.getASTContext(), AST.getTokens(), R.Begin,
- R.End);
+ auto Selection = SelectionTree::createRight(
+ AST.getASTContext(), AST.getTokens(), R.Begin, R.End);
const SelectionTree::Node *N = Selection.commonAncestor();
if (!N) {
ADD_FAILURE() << "No node selected!\n" << Code;
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 783599ad7ac0..52abc3ec4db9 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -568,7 +568,7 @@ TEST(Hover, NoHover) {
}
)cpp",
R"cpp(// Template auto parameter. Nothing (Not useful).
- template<^auto T>
+ template<a^uto T>
void func() {
}
void foo() {
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 9e1a90b55e3a..4af267b6b3ed 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -19,20 +19,26 @@ namespace clangd {
namespace {
using ::testing::UnorderedElementsAreArray;
+// Create a selection tree corresponding to a point or pair of points.
+// This uses the precisely-defined createRight semantics. The fuzzier
+// createEach is tested separately.
SelectionTree makeSelectionTree(const StringRef MarkedCode, ParsedAST &AST) {
Annotations Test(MarkedCode);
switch (Test.points().size()) {
- case 1: // Point selection.
- return SelectionTree(AST.getASTContext(), AST.getTokens(),
- cantFail(positionToOffset(Test.code(), Test.point())));
+ case 1: { // Point selection.
+ unsigned Offset = cantFail(positionToOffset(Test.code(), Test.point()));
+ return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+ Offset, Offset);
+ }
case 2: // Range selection.
- return SelectionTree(
+ return SelectionTree::createRight(
AST.getASTContext(), AST.getTokens(),
cantFail(positionToOffset(Test.code(), Test.points()[0])),
cantFail(positionToOffset(Test.code(), Test.points()[1])));
default:
ADD_FAILURE() << "Expected 1-2 points for selection.\n" << MarkedCode;
- return SelectionTree(AST.getASTContext(), AST.getTokens(), 0u, 0u);
+ return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0u,
+ 0u);
}
}
@@ -507,6 +513,61 @@ TEST(SelectionTest, Implicit) {
EXPECT_EQ("CXXConstructExpr", nodeKind(&Str->outerImplicit()));
}
+TEST(SelectionTest, CreateAll) {
+ llvm::Annotations Test("int$unique^ a=1$ambiguous^+1; $empty^");
+ auto AST = TestTU::withCode(Test.code()).build();
+ unsigned Seen = 0;
+ SelectionTree::createEach(
+ AST.getASTContext(), AST.getTokens(), Test.point("ambiguous"),
+ Test.point("ambiguous"), [&](SelectionTree T) {
+ // Expect to see the right-biased tree first.
+ if (Seen == 0)
+ EXPECT_EQ("BinaryOperator", nodeKind(T.commonAncestor()));
+ else if (Seen == 1)
+ EXPECT_EQ("IntegerLiteral", nodeKind(T.commonAncestor()));
+ ++Seen;
+ return false;
+ });
+ EXPECT_EQ(2u, Seen);
+
+ Seen = 0;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(),
+ Test.point("ambiguous"), Test.point("ambiguous"),
+ [&](SelectionTree T) {
+ ++Seen;
+ return true;
+ });
+ EXPECT_EQ(1u, Seen) << "Return true --> stop iterating";
+
+ Seen = 0;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(),
+ Test.point("unique"), Test.point("unique"),
+ [&](SelectionTree T) {
+ ++Seen;
+ return false;
+ });
+ EXPECT_EQ(1u, Seen) << "no ambiguity --> only one tree";
+
+ Seen = 0;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(),
+ Test.point("empty"), Test.point("empty"),
+ [&](SelectionTree T) {
+ EXPECT_FALSE(T.commonAncestor());
+ ++Seen;
+ return false;
+ });
+ EXPECT_EQ(1u, Seen) << "empty tree still created";
+
+ Seen = 0;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(),
+ Test.point("unique"), Test.point("ambiguous"),
+ [&](SelectionTree T) {
+ ++Seen;
+ return false;
+ });
+ EXPECT_EQ(1u, Seen) << "one tree for nontrivial selection";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
index 7f9f75c08198..587c07da0e28 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,12 +63,33 @@ std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
cantFail(positionToOffset(A.code(), SelectionRng.end))};
}
+// Prepare and apply the specified tweak based on the selection in Input.
+// Returns None if and only if prepare() failed.
+llvm::Optional<llvm::Expected<Tweak::Effect>>
+applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
+ const SymbolIndex *Index) {
+ auto Range = rangeOrPoint(Input);
+ llvm::Optional<llvm::Expected<Tweak::Effect>> Result;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
+ Range.second, [&](SelectionTree ST) {
+ Tweak::Selection S(Index, AST, Range.first,
+ Range.second, std::move(ST));
+ if (auto T = prepareTweak(TweakID, S)) {
+ Result = (*T)->apply(S);
+ return true;
+ } else {
+ llvm::consumeError(T.takeError());
+ return false;
+ }
+ });
+ return Result;
+}
+
MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
std::string WrappedCode = wrap(Ctx, arg);
Annotations Input(WrappedCode);
- auto Selection = rangeOrPoint(Input);
TestTU TU;
TU.Filename = FileName;
TU.HeaderCode = Header;
@@ -76,12 +97,11 @@ MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
TU.ExtraArgs = ExtraArgs;
TU.AdditionalFiles = std::move(ExtraFiles);
ParsedAST AST = TU.build();
- Tweak::Selection S(Index, AST, Selection.first, Selection.second);
- auto PrepareResult = prepareTweak(TweakID, S);
- if (PrepareResult)
- return true;
- llvm::consumeError(PrepareResult.takeError());
- return false;
+ auto Result = applyTweak(AST, Input, TweakID, Index);
+ // We only care if prepare() succeeded, but must handle Errors.
+ if (Result && !*Result)
+ consumeError(Result->takeError());
+ return Result.hasValue();
}
} // namespace
@@ -90,8 +110,6 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
llvm::StringMap<std::string> *EditedFiles) const {
std::string WrappedCode = wrap(Context, MarkedCode);
Annotations Input(WrappedCode);
- auto Selection = rangeOrPoint(Input);
-
TestTU TU;
TU.Filename = FileName;
TU.HeaderCode = Header;
@@ -99,23 +117,20 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
TU.Code = Input.code();
TU.ExtraArgs = ExtraArgs;
ParsedAST AST = TU.build();
- Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
- auto T = prepareTweak(TweakID, S);
- if (!T) {
- llvm::consumeError(T.takeError());
- return "unavailable";
- }
- llvm::Expected<Tweak::Effect> Result = (*T)->apply(S);
+ auto Result = applyTweak(AST, Input, TweakID, Index.get());
if (!Result)
- return "fail: " + llvm::toString(Result.takeError());
- if (Result->ShowMessage)
- return "message:\n" + *Result->ShowMessage;
- if (Result->ApplyEdits.empty())
+ return "unavailable";
+ if (!*Result)
+ return "fail: " + llvm::toString(Result->takeError());
+ const auto &Effect = **Result;
+ if ((*Result)->ShowMessage)
+ return "message:\n" + *Effect.ShowMessage;
+ if (Effect.ApplyEdits.empty())
return "no effect";
std::string EditedMainFile;
- for (auto &It : Result->ApplyEdits) {
+ for (auto &It : Effect.ApplyEdits) {
auto NewText = It.second.apply();
if (!NewText)
return "bad edits: " + llvm::toString(NewText.takeError());
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 8627439d4555..b6734b72380e 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1971,7 +1971,7 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
// Basic check for function body and signature.
EXPECT_AVAILABLE(R"cpp(
class Bar {
- [[void [[f^o^o]]() [[{ return; }]]]]
+ [[void [[f^o^o^]]() [[{ return; }]]]]
};
void foo();
More information about the cfe-commits
mailing list