diff --git a/clang-tools-extra/clangd/Selection.h b/clang-tools-extra/clangd/Selection.h --- a/clang-tools-extra/clangd/Selection.h +++ b/clang-tools-extra/clangd/Selection.h @@ -98,11 +98,9 @@ const DeclContext& getDeclContext() const; }; // The most specific common ancestor of all the selected nodes. - // If there is no selection, this is nullptr. - const Node *commonAncestor() const; + const Node &commonAncestor() const; // The selection node corresponding to TranslationUnitDecl. - // If there is no selection, this is nullptr. - const Node *root() const { return Root; } + const Node &root() const { return *Root; } private: std::deque Nodes; // Stable-pointer storage. @@ -112,10 +110,7 @@ void print(llvm::raw_ostream &OS, const Node &N, int Indent) const; friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SelectionTree &T) { - if (auto R = T.root()) - T.print(OS, *R, 0); - else - OS << "(empty selection)\n"; + T.print(OS, T.root(), 1); return OS; } }; diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -106,8 +106,14 @@ V.TraverseAST(AST); assert(V.Stack.size() == 1 && "Unpaired push/pop?"); assert(V.Stack.top() == &V.Nodes.front()); - if (V.Nodes.size() == 1) // TUDecl, but no nodes under it. - V.Nodes.clear(); + // We selected TUDecl if characters were unclaimed (or the file is empty). + if (V.Nodes.size() == 1 || V.Claimed.add(Begin, End)) { + StringRef FileContent = AST.getSourceManager().getBufferData(File); + // Don't require the trailing newlines to be selected. + bool SelectedAll = Begin == 0 && End >= FileContent.rtrim().size(); + V.Stack.top()->Selected = + SelectedAll ? SelectionTree::Complete : SelectionTree::Partial; + } return std::move(V.Nodes); } @@ -408,13 +414,11 @@ SelectionTree::SelectionTree(ASTContext &AST, unsigned Offset) : SelectionTree(AST, Offset, Offset) {} -const Node *SelectionTree::commonAncestor() const { - if (!Root) - return nullptr; +const Node &SelectionTree::commonAncestor() const { const Node *Ancestor = Root; while (Ancestor->Children.size() == 1 && !Ancestor->Selected) Ancestor = Ancestor->Children.front(); - return Ancestor; + return *Ancestor; } const DeclContext& SelectionTree::Node::getDeclContext() const { diff --git a/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp b/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp @@ -24,7 +24,7 @@ const char *id() const override final; bool prepare(const Selection &Inputs) override { - for (auto N = Inputs.ASTSelection.commonAncestor(); N && !InterestedDecl; + for (auto *N = &Inputs.ASTSelection.commonAncestor(); N && !InterestedDecl; N = N->Parent) InterestedDecl = N->ASTNode.get(); return InterestedDecl; diff --git a/clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp b/clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp @@ -33,9 +33,9 @@ const char *id() const override final; bool prepare(const Selection &Inputs) override { - for (auto N = Inputs.ASTSelection.commonAncestor(); N && !Node; + for (auto *N = &Inputs.ASTSelection.commonAncestor(); N && !Node; N = N->Parent) - if (dumpable(N->ASTNode)) + if (N->Parent && dumpable(N->ASTNode)) Node = N->ASTNode; return Node.hasValue(); } @@ -85,7 +85,7 @@ const char *id() const override final; bool prepare(const Selection &Inputs) override { - return Inputs.ASTSelection.root() != nullptr; + return Inputs.ASTSelection.commonAncestor().Parent != nullptr; } Expected apply(const Selection &Inputs) override { return Effect::showMessage(llvm::to_string(Inputs.ASTSelection)); @@ -110,9 +110,8 @@ const char *id() const override final; bool prepare(const Selection &Inputs) override { - if (auto *Node = Inputs.ASTSelection.commonAncestor()) - if (auto *D = Node->ASTNode.get()) - Record = dyn_cast(D); + if (auto *D = Inputs.ASTSelection.commonAncestor().ASTNode.get()) + Record = dyn_cast(D); return Record && Record->isThisDeclarationADefinition() && !Record->isDependentType(); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp @@ -58,7 +58,7 @@ bool ExpandAutoType::prepare(const Selection& Inputs) { CachedLocation = llvm::None; - if (auto *Node = Inputs.ASTSelection.commonAncestor()) { + if (auto *Node = &Inputs.ASTSelection.commonAncestor()) { if (auto *TypeNode = Node->ASTNode.get()) { if (const AutoTypeLoc Result = TypeNode->getAs()) { CachedLocation = Result; @@ -94,8 +94,8 @@ Inputs); } - std::string PrettyTypeName = printType(*DeducedType, - Inputs.ASTSelection.commonAncestor()->getDeclContext()); + std::string PrettyTypeName = printType( + *DeducedType, Inputs.ASTSelection.commonAncestor().getDeclContext()); tooling::Replacement Expansion(SrcMgr, CharSourceRange(CachedLocation->getSourceRange(), true), diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -33,10 +33,10 @@ // information regarding the Expr that is being extracted class ExtractionContext { public: - ExtractionContext(const SelectionTree::Node *Node, const SourceManager &SM, + ExtractionContext(const SelectionTree::Node &Node, const SourceManager &SM, const ASTContext &Ctx); const clang::Expr *getExpr() const { return Expr; } - const SelectionTree::Node *getExprNode() const { return ExprNode; } + const SelectionTree::Node &getExprNode() const { return ExprNode; } bool isExtractable() const { return Extractable; } // Generate Replacement for replacing selected expression with given VarName tooling::Replacement replaceWithVar(llvm::StringRef VarName) const; @@ -46,7 +46,7 @@ private: bool Extractable = false; const clang::Expr *Expr; - const SelectionTree::Node *ExprNode; + const SelectionTree::Node &ExprNode; // Stmt before which we will extract const clang::Stmt *InsertionPoint = nullptr; const SourceManager &SM; @@ -89,11 +89,11 @@ return false; } -ExtractionContext::ExtractionContext(const SelectionTree::Node *Node, +ExtractionContext::ExtractionContext(const SelectionTree::Node &Node, const SourceManager &SM, const ASTContext &Ctx) : ExprNode(Node), SM(SM), Ctx(Ctx) { - Expr = Node->ASTNode.get(); + Expr = Node.ASTNode.get(); if (isExtractableExpr(Expr)) { ReferencedDecls = computeReferencedDecls(Expr); InsertionPoint = computeInsertionPoint(); @@ -148,7 +148,7 @@ return true; return false; }; - for (const SelectionTree::Node *CurNode = getExprNode(); + for (const SelectionTree::Node *CurNode = &getExprNode(); CurNode->Parent && CanExtractOutside(CurNode); CurNode = CurNode->Parent) { const clang::Stmt *CurInsertionPoint = CurNode->ASTNode.get(); @@ -216,13 +216,12 @@ }; REGISTER_TWEAK(ExtractVariable) bool ExtractVariable::prepare(const Selection &Inputs) { - const ASTContext &Ctx = Inputs.AST.getASTContext(); - const SourceManager &SM = Inputs.AST.getSourceManager(); - const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); // we don't trigger on empty selections for now - if (!N || Inputs.SelectionBegin == Inputs.SelectionEnd) + if (Inputs.SelectionBegin == Inputs.SelectionEnd) return false; - Target = llvm::make_unique(N, SM, Ctx); + Target = llvm::make_unique( + Inputs.ASTSelection.commonAncestor(), Inputs.AST.getSourceManager(), + Inputs.AST.getASTContext()); return Target->isExtractable(); } diff --git a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp @@ -78,10 +78,8 @@ } bool RawStringLiteral::prepare(const Selection &Inputs) { - const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); - if (!N) - return false; - Str = dyn_cast_or_null(N->ASTNode.get()); + const SelectionTree::Node &N = Inputs.ASTSelection.commonAncestor(); + Str = dyn_cast_or_null(N.ASTNode.get()); return Str && isNormalString(*Str, Inputs.Cursor, Inputs.AST.getSourceManager()) && needsRaw(Str->getBytes()) && canBeRaw(Str->getBytes()); diff --git a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp @@ -48,7 +48,7 @@ REGISTER_TWEAK(SwapIfBranches) bool SwapIfBranches::prepare(const Selection &Inputs) { - for (const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); + for (const SelectionTree::Node *N = &Inputs.ASTSelection.commonAncestor(); N && !If; N = N->Parent) { // Stop once we hit a block, e.g. a lambda in the if condition. if (dyn_cast_or_null(N->ASTNode.get())) diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -9,6 +9,8 @@ #include "Selection.h" #include "SourceCode.h" #include "TestTU.h" +#include "clang/AST/Decl.h" +#include "llvm/Support/Casting.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -34,28 +36,26 @@ } } -Range nodeRange(const SelectionTree::Node *N, ParsedAST &AST) { - if (!N) - return Range{}; +Range nodeRange(const SelectionTree::Node &N, ParsedAST &AST) { const SourceManager &SM = AST.getSourceManager(); const LangOptions &LangOpts = AST.getASTContext().getLangOpts(); StringRef Buffer = SM.getBufferData(SM.getMainFileID()); + if (llvm::isa_and_nonnull(N.ASTNode.get())) + return Range{Position{}, offsetToPosition(Buffer, Buffer.size())}; auto FileRange = - toHalfOpenFileRange(SM, LangOpts, N->ASTNode.getSourceRange()); + toHalfOpenFileRange(SM, LangOpts, N.ASTNode.getSourceRange()); assert(FileRange && "We should be able to get the File Range"); return Range{ offsetToPosition(Buffer, SM.getFileOffset(FileRange->getBegin())), offsetToPosition(Buffer, SM.getFileOffset(FileRange->getEnd()))}; } -std::string nodeKind(const SelectionTree::Node *N) { - if (!N) - return ""; - return N->ASTNode.getNodeKind().asStringRef().str(); +std::string nodeKind(const SelectionTree::Node &N) { + return N.ASTNode.getNodeKind().asStringRef().str(); } std::vector allNodes(const SelectionTree &T) { - std::vector Result = {T.root()}; + std::vector Result = {&T.root()}; for (unsigned I = 0; I < Result.size(); ++I) { const SelectionTree::Node *N = Result[I]; Result.insert(Result.end(), N->Children.begin(), N->Children.end()); @@ -65,16 +65,16 @@ // Returns true if Common is a descendent of Root. // Verifies nothing is selected above Common. -bool verifyCommonAncestor(const SelectionTree::Node *Root, - const SelectionTree::Node *Common, +bool verifyCommonAncestor(const SelectionTree::Node &Root, + const SelectionTree::Node &Common, StringRef MarkedCode) { - if (Root == Common) + if (&Root == &Common) return true; - if (Root->Selected) + if (Root.Selected) ADD_FAILURE() << "Selected nodes outside common ancestor\n" << MarkedCode; bool Seen = false; - for (const SelectionTree::Node *Child : Root->Children) - if (verifyCommonAncestor(Child, Common, MarkedCode)) { + for (const SelectionTree::Node *Child : Root.Children) + if (verifyCommonAncestor(*Child, Common, MarkedCode)) { if (Seen) ADD_FAILURE() << "Saw common ancestor twice\n" << MarkedCode; Seen = true; @@ -162,7 +162,7 @@ #define CALL_FUNCTION(X) X^()^ void bar() { CALL_FUNCTION(foo); } )cpp", - nullptr, + "TranslationUnitDecl", }, { R"cpp( @@ -230,13 +230,13 @@ {"[[struct {int x;} ^y]];", "VarDecl"}, {"struct {[[int ^x]];} y;", "FieldDecl"}, - {"^", nullptr}, + {"^", "TranslationUnitDecl"}, {"void foo() { [[foo^^]] (); }", "DeclRefExpr"}, // FIXME: Ideally we'd get a declstmt or the VarDecl itself here. // This doesn't happen now; the RAV doesn't traverse a node containing ;. - {"int x = 42;^", nullptr}, - {"int x = 42^;", nullptr}, + {"int x = 42;^", "TranslationUnitDecl"}, + {"int x = 42^;", "TranslationUnitDecl"}, // Node types that have caused problems in the past. {"template void foo() { [[^T]] t; }", "TypeLoc"}, @@ -257,8 +257,7 @@ if (Test.ranges().empty()) { // If no [[range]] is marked in the example, there should be no selection. - EXPECT_FALSE(T.commonAncestor()) << C.Code << "\n" << T; - EXPECT_FALSE(T.root()) << C.Code << "\n" << T; + EXPECT_EQ(&T.root(), &T.commonAncestor()) << C.Code << "\n" << T; } else { // If there is an expected selection, both common ancestor and root // should exist with the appropriate node types in them. @@ -285,7 +284,7 @@ auto AST = TestTU::withCode(Annotations(Code).code()).build(); auto T = makeSelectionTree(Code, AST); ASSERT_EQ("CXXRecordDecl", nodeKind(T.commonAncestor())) << T; - auto *D = dyn_cast(T.commonAncestor()->ASTNode.get()); + auto *D = dyn_cast(T.commonAncestor().ASTNode.get()); EXPECT_FALSE(D->isInjectedClassName()); } @@ -314,10 +313,10 @@ void foo(^$C[[unique_ptr<$C[[unique_ptr<$C[[int]]>]]>]]^ a) {} )cpp", R"cpp(int a = [[5 >^> 1]];)cpp", - R"cpp( + R"cpp([[ #define ECHO(X) X ECHO(EC^HO([[$C[[int]]) EC^HO(a]])); - )cpp", + ]])cpp", }; for (const char *C : Cases) { Annotations Test(C); @@ -327,9 +326,9 @@ std::vector Complete, Partial; for (const SelectionTree::Node *N : allNodes(T)) if (N->Selected == SelectionTree::Complete) - Complete.push_back(nodeRange(N, AST)); + Complete.push_back(nodeRange(*N, AST)); else if (N->Selected == SelectionTree::Partial) - Partial.push_back(nodeRange(N, AST)); + Partial.push_back(nodeRange(*N, AST)); EXPECT_THAT(Complete, UnorderedElementsAreArray(Test.ranges("C"))) << C; EXPECT_THAT(Partial, UnorderedElementsAreArray(Test.ranges())) << C; } diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -269,13 +269,13 @@ checkNotAvailable(ID, "/*c^omment*/ int foo() return 2 ^ + 2; }"); const char *Input = "int fcall(int); int x = fca[[ll(2 +]]2);"; - const char *Output = R"(TranslationUnitDecl - VarDecl int x = fcall(2 + 2) - .CallExpr fcall(2 + 2) - ImplicitCastExpr fcall - .DeclRefExpr fcall - .BinaryOperator 2 + 2 - *IntegerLiteral 2 + const char *Output = R"( TranslationUnitDecl + VarDecl int x = fcall(2 + 2) + .CallExpr fcall(2 + 2) + ImplicitCastExpr fcall + .DeclRefExpr fcall + .BinaryOperator 2 + 2 + *IntegerLiteral 2 )"; EXPECT_EQ(Output, getMessage(ID, Input)); }