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(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
