sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Previously TranslationUnitDecl would never be selected.
This means root() and commonAncestor() are now never null, and therefore changed
them to references.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65101
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/Selection.h
clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ 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));
}
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ 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<TranslationUnitDecl>(N.ASTNode.get<Decl>()))
+ 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 "<null>";
- return N->ASTNode.getNodeKind().asStringRef().str();
+std::string nodeKind(const SelectionTree::Node &N) {
+ return N.ASTNode.getNodeKind().asStringRef().str();
}
std::vector<const SelectionTree::Node *> allNodes(const SelectionTree &T) {
- std::vector<const SelectionTree::Node *> Result = {T.root()};
+ std::vector<const SelectionTree::Node *> 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 <typename T> 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<CXXRecordDecl>(T.commonAncestor()->ASTNode.get<Decl>());
+ auto *D = dyn_cast<CXXRecordDecl>(T.commonAncestor().ASTNode.get<Decl>());
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<Range> 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;
}
Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ 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<CompoundStmt>(N->ASTNode.get<Stmt>()))
Index: clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
+++ 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<StringLiteral>(N->ASTNode.get<Stmt>());
+ const SelectionTree::Node &N = Inputs.ASTSelection.commonAncestor();
+ Str = dyn_cast_or_null<StringLiteral>(N.ASTNode.get<Stmt>());
return Str &&
isNormalString(*Str, Inputs.Cursor, Inputs.AST.getSourceManager()) &&
needsRaw(Str->getBytes()) && canBeRaw(Str->getBytes());
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ 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<clang::Expr>();
+ Expr = Node.ASTNode.get<clang::Expr>();
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<Stmt>();
@@ -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<ExtractionContext>(N, SM, Ctx);
+ Target = llvm::make_unique<ExtractionContext>(
+ Inputs.ASTSelection.commonAncestor(), Inputs.AST.getSourceManager(),
+ Inputs.AST.getASTContext());
return Target->isExtractable();
}
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ 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<TypeLoc>()) {
if (const AutoTypeLoc Result = TypeNode->getAs<AutoTypeLoc>()) {
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),
Index: clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
+++ 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<Effect> 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<Decl>())
- Record = dyn_cast<RecordDecl>(D);
+ if (auto *D = Inputs.ASTSelection.commonAncestor().ASTNode.get<Decl>())
+ Record = dyn_cast<RecordDecl>(D);
return Record && Record->isThisDeclarationADefinition() &&
!Record->isDependentType();
}
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ 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<Decl>();
return InterestedDecl;
Index: clang-tools-extra/clangd/Selection.h
===================================================================
--- clang-tools-extra/clangd/Selection.h
+++ 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<Node> 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;
}
};
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ 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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
