[clang-tools-extra] b4b9706 - Revert "[clangd] Reapply b60896fad926 Fall back to selecting token-before-cursor if token-after-cursor fails."
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 23 07:36:04 PST 2020
Author: Sam McCall
Date: 2020-02-23T16:34:49+01:00
New Revision: b4b9706d5da368c81b86867b1c11a2e17b4472ac
URL: https://github.com/llvm/llvm-project/commit/b4b9706d5da368c81b86867b1c11a2e17b4472ac
DIFF: https://github.com/llvm/llvm-project/commit/b4b9706d5da368c81b86867b1c11a2e17b4472ac.diff
LOG: Revert "[clangd] Reapply b60896fad926 Fall back to selecting token-before-cursor if token-after-cursor fails."
This reverts commit a2ce807eb72a8e154abca09b1e968b2d99ba6933.
Buildbot failures on GCC due to SelectionTree not being copyable, and
instantiating vector<Selection> in the tweak-handling in ClangdServer.
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 716983a7a227..48cf921ff18c 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -395,8 +395,7 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
WorkScheduler.runWithAST("Rename", File, std::move(Action));
}
-// May generate several candidate selections, due to SelectionTree ambiguity.
-static llvm::Expected<std::vector<Tweak::Selection>>
+static llvm::Expected<Tweak::Selection>
tweakSelection(const Range &Sel, const InputsAndAST &AST) {
auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
if (!Begin)
@@ -404,15 +403,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
if (!End)
return End.takeError();
- 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;
+ return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
}
void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
@@ -421,21 +412,12 @@ void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
this](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- auto Selections = tweakSelection(Sel, *InpAST);
- if (!Selections)
- return CB(Selections.takeError());
+ auto Selection = tweakSelection(Sel, *InpAST);
+ if (!Selection)
+ return CB(Selection.takeError());
std::vector<TweakRef> Res;
- // 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());
- }
- }
+ for (auto &T : prepareTweaks(*Selection, TweakFilter))
+ Res.push_back({T->id(), T->title(), T->intent()});
CB(std::move(Res));
};
@@ -450,30 +432,21 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
- 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));
- }
+ 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));
}
return CB(std::move(*Effect));
};
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 851d742711e5..750df50c4777 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -537,12 +537,9 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
llvm::consumeError(Offset.takeError());
return llvm::None;
}
- // 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);
+ SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
std::vector<const Decl *> Result;
- if (const SelectionTree::Node *N = ST.commonAncestor()) {
+ if (const SelectionTree::Node *N = Selection.commonAncestor()) {
auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
if (!Decls.empty()) {
HI = getHoverContents(Decls.front(), Index);
diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index d1438d134e15..574b7477f23f 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -142,11 +142,6 @@ 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.
@@ -177,7 +172,9 @@ class SelectionTester {
});
// Precompute selectedness and offset for selected spelled tokens.
for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) {
- if (shouldIgnore(*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)
continue;
SpelledTokens.emplace_back();
Tok &S = SpelledTokens.back();
@@ -674,49 +671,24 @@ std::string SelectionTree::Node::kind() const {
return std::move(OS.str());
}
-// 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);
+// 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};
}
SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
@@ -726,6 +698,8 @@ 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;
@@ -737,6 +711,10 @@ 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 36196f812db9..a7050c49be6b 100644
--- a/clang-tools-extra/clangd/Selection.h
+++ b/clang-tools-extra/clangd/Selection.h
@@ -29,14 +29,6 @@
// - 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
@@ -72,32 +64,16 @@ namespace clangd {
// point back into the AST it was constructed with.
class SelectionTree {
public:
- // 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;
+ // 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);
// Describes to what extent an AST node is covered by the selection.
enum Selection : unsigned char {
@@ -145,11 +121,6 @@ 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 49fc7703db05..cbbf31f1b05b 100644
--- a/clang-tools-extra/clangd/SemanticSelection.cpp
+++ b/clang-tools-extra/clangd/SemanticSelection.cpp
@@ -39,8 +39,7 @@ llvm::Expected<std::vector<Range>> getSemanticRanges(ParsedAST &AST,
}
// Get node under the cursor.
- SelectionTree ST = SelectionTree::createRight(
- AST.getASTContext(), AST.getTokens(), *Offset, *Offset);
+ SelectionTree ST(AST.getASTContext(), AST.getTokens(), *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 c723f97dc501..61dff3e99cae 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -134,16 +134,15 @@ SymbolLocation getPreferredLocation(const Location &ASTLoc,
std::vector<const NamedDecl *> getDeclAtPosition(ParsedAST &AST,
SourceLocation Pos,
DeclRelationSet Relations) {
- unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Pos).second;
+ FileID FID;
+ unsigned Offset;
+ std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+ SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
std::vector<const NamedDecl *> Result;
- 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();
- });
+ if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+ auto Decls = targetDecl(N->ASTNode, Relations);
+ Result.assign(Decls.begin(), Decls.end());
+ }
return Result;
}
@@ -713,50 +712,41 @@ static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
}
const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) {
- auto RecordFromNode =
- [](const SelectionTree::Node *N) -> const CXXRecordDecl * {
- if (!N)
- return nullptr;
-
- // Note: explicitReferenceTargets() will search for both template
- // instantiations and template patterns, and prefer the former if available
- // (generally, one will be available for non-dependent specializations of a
- // class template).
- auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Underlying);
- if (Decls.empty())
- return nullptr;
-
- const NamedDecl *D = Decls[0];
-
- if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
- // If this is a variable, use the type of the variable.
- return VD->getType().getTypePtr()->getAsCXXRecordDecl();
- }
+ const SourceManager &SM = AST.getSourceManager();
+ SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
+ getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
+ unsigned Offset =
+ AST.getSourceManager().getDecomposedSpellingLoc(SourceLocationBeg).second;
+ SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+ const SelectionTree::Node *N = Selection.commonAncestor();
+ if (!N)
+ return nullptr;
- if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
- // If this is a method, use the type of the class.
- return Method->getParent();
- }
+ // Note: explicitReferenceTargets() will search for both template
+ // instantiations and template patterns, and prefer the former if available
+ // (generally, one will be available for non-dependent specializations of a
+ // class template).
+ auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Underlying);
+ if (Decls.empty())
+ return nullptr;
- // We don't handle FieldDecl because it's not clear what behaviour
- // the user would expect: the enclosing class type (as with a
- // method), or the field's type (as with a variable).
+ const NamedDecl *D = Decls[0];
- return dyn_cast<CXXRecordDecl>(D);
- };
+ if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+ // If this is a variable, use the type of the variable.
+ return VD->getType().getTypePtr()->getAsCXXRecordDecl();
+ }
- const SourceManager &SM = AST.getSourceManager();
- SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
- unsigned Offset = SM.getDecomposedSpellingLoc(SourceLocationBeg).second;
- const CXXRecordDecl *Result = nullptr;
- SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
- Offset, [&](SelectionTree ST) {
- Result = RecordFromNode(ST.commonAncestor());
- return Result != nullptr;
- });
- return Result;
+ if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
+ // If this is a method, use the type of the class.
+ return Method->getParent();
+ }
+
+ // We don't handle FieldDecl because it's not clear what behaviour
+ // the user would expect: the enclosing class type (as with a
+ // method), or the field's type (as with a variable).
+ return dyn_cast<CXXRecordDecl>(D);
}
std::vector<const CXXRecordDecl *> typeParents(const CXXRecordDecl *CXXRD) {
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 91436431ba09..02e355ecf164 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -81,8 +81,7 @@ llvm::DenseSet<const NamedDecl *> locateDeclAt(ParsedAST &AST,
unsigned Offset =
AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
- SelectionTree Selection = SelectionTree::createRight(
- AST.getASTContext(), AST.getTokens(), Offset, Offset);
+ SelectionTree Selection(AST.getASTContext(), AST.getTokens(), 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 3e3033ce5c7a..435c36e91efa 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,
- SelectionTree ASTSelection)
+ unsigned RangeBegin, unsigned RangeEnd)
: Index(Index), AST(&AST), SelectionBegin(RangeBegin),
- SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
+ SelectionEnd(RangeEnd),
+ ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) {
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 84d3bdb3fc0a..ca4d43dfa0cc 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, SelectionTree ASTSelection);
+ unsigned RangeEnd);
/// 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 c38ccc3f9441..5cf94b6ff6b0 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -76,8 +76,8 @@ class TargetDeclTest : public ::testing::Test {
TU.ExtraArgs = Flags;
auto AST = TU.build();
llvm::Annotations::Range R = A.range();
- auto Selection = SelectionTree::createRight(
- AST.getASTContext(), AST.getTokens(), R.Begin, R.End);
+ SelectionTree Selection(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 e4de8af93e9e..503b4d2afa42 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -654,7 +654,7 @@ TEST(Hover, NoHover) {
}
)cpp",
R"cpp(// Template auto parameter. Nothing (Not useful).
- template<a^uto T>
+ template<^auto T>
void func() {
}
void foo() {
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index cf19e07f0b1a..c74ce4426fd0 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -19,26 +19,20 @@ 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.
- unsigned Offset = cantFail(positionToOffset(Test.code(), Test.point()));
- return SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
- Offset, Offset);
- }
+ case 1: // Point selection.
+ return SelectionTree(AST.getASTContext(), AST.getTokens(),
+ cantFail(positionToOffset(Test.code(), Test.point())));
case 2: // Range selection.
- return SelectionTree::createRight(
+ return SelectionTree(
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::createRight(AST.getASTContext(), AST.getTokens(), 0u,
- 0u);
+ return SelectionTree(AST.getASTContext(), AST.getTokens(), 0u, 0u);
}
}
@@ -560,61 +554,6 @@ 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 bbb1af46f93c..33779f4f1407 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,33 +63,12 @@ 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 = std::string(FileName);
TU.HeaderCode = Header;
@@ -97,11 +76,12 @@ MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
TU.ExtraArgs = ExtraArgs;
TU.AdditionalFiles = std::move(ExtraFiles);
ParsedAST AST = TU.build();
- 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();
+ Tweak::Selection S(Index, AST, Selection.first, Selection.second);
+ auto PrepareResult = prepareTweak(TweakID, S);
+ if (PrepareResult)
+ return true;
+ llvm::consumeError(PrepareResult.takeError());
+ return false;
}
} // namespace
@@ -110,6 +90,8 @@ 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 = std::string(FileName);
TU.HeaderCode = Header;
@@ -117,20 +99,23 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
TU.Code = std::string(Input.code());
TU.ExtraArgs = ExtraArgs;
ParsedAST AST = TU.build();
+ Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
- auto Result = applyTweak(AST, Input, TweakID, Index.get());
- if (!Result)
+ auto T = prepareTweak(TweakID, S);
+ if (!T) {
+ llvm::consumeError(T.takeError());
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())
+ }
+ llvm::Expected<Tweak::Effect> Result = (*T)->apply(S);
+ if (!Result)
+ return "fail: " + llvm::toString(Result.takeError());
+ if (Result->ShowMessage)
+ return "message:\n" + *Result->ShowMessage;
+ if (Result->ApplyEdits.empty())
return "no effect";
std::string EditedMainFile;
- for (auto &It : Effect.ApplyEdits) {
+ for (auto &It : Result->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 40ab6b12643e..68f87a71daf5 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1966,7 +1966,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