[clang-tools-extra] 20f8f46 - [clangd] Fix selection on multi-dimensional array.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 4 03:06:21 PST 2022
Author: Haojian Wu
Date: 2022-01-04T11:53:42+01:00
New Revision: 20f8f46c60b39fb2c6b4371a03e580d0711e8d82
URL: https://github.com/llvm/llvm-project/commit/20f8f46c60b39fb2c6b4371a03e580d0711e8d82
DIFF: https://github.com/llvm/llvm-project/commit/20f8f46c60b39fb2c6b4371a03e580d0711e8d82.diff
LOG: [clangd] Fix selection on multi-dimensional array.
This involves separating out the concepts of "which tokens should we
descend into this node for" vs "which tokens should this node claim".
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D116218
Added:
Modified:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 0b10c7a3a6f9..2024228e2b58 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -58,6 +58,7 @@ void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
SelectionUsedRecovery.record(0, LanguageLabel); // unused.
}
+// Return the range covering a node and all its children.
SourceRange getSourceRange(const DynTypedNode &N) {
// MemberExprs to implicitly access anonymous fields should not claim any
// tokens for themselves. Given:
@@ -702,7 +703,7 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
void pop() {
Node &N = *Stack.top();
dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
- claimRange(getSourceRange(N.ASTNode), N.Selected);
+ claimTokensFor(N.ASTNode, N.Selected);
if (N.Selected == NoTokens)
N.Selected = SelectionTree::Unselected;
if (N.Selected || !N.Children.empty()) {
@@ -744,6 +745,28 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
return SourceRange();
}
+ // Claim tokens for N, after processing its children.
+ // By default this claims all unclaimed tokens in getSourceRange().
+ // We override this if we want to claim fewer tokens (e.g. there are gaps).
+ void claimTokensFor(const DynTypedNode &N, SelectionTree::Selection &Result) {
+ if (const auto *TL = N.get<TypeLoc>()) {
+ // e.g. EltType Foo[OuterSize][InnerSize];
+ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ArrayTypeLoc (Outer)
+ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |-ArrayTypeLoc (Inner)
+ // ~~~~~~~ | |-RecordType
+ // ~~~~~~~~~ | `-Expr (InnerSize)
+ // ~~~~~~~~~ `-Expr (OuterSize)
+ // Inner ATL must not claim its whole SourceRange, or it clobbers Outer.
+ if (TL->getAs<ArrayTypeLoc>()) {
+ claimRange(TL->getLocalSourceRange(), Result);
+ return;
+ }
+ // FIXME: maybe LocalSourceRange is a better default for TypeLocs.
+ // It doesn't seem to be usable for FunctionTypeLocs.
+ }
+ claimRange(getSourceRange(N), Result);
+ }
+
// Perform hit-testing of a complete Node against the selection.
// This runs for every node in the AST, and must be fast in common cases.
// This is usually called from pop(), so we can take children into account.
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 6c6782a097db..971487c9cd27 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -318,6 +318,14 @@ TEST(SelectionTest, CommonAncestor) {
{"[[st^ruct {int x;}]] y;", "CXXRecordDecl"},
{"[[struct {int x;} ^y]];", "VarDecl"},
{"struct {[[int ^x]];} y;", "FieldDecl"},
+
+ // Tricky case: nested ArrayTypeLocs have the same token range.
+ {"const int x = 1, y = 2; int array[^[[x]]][10][y];", "DeclRefExpr"},
+ {"const int x = 1, y = 2; int array[x][10][^[[y]]];", "DeclRefExpr"},
+ {"const int x = 1, y = 2; int array[x][^[[10]]][y];", "IntegerLiteral"},
+ {"const int x = 1, y = 2; [[i^nt]] array[x][10][y];", "BuiltinTypeLoc"},
+ {"void func(int x) { int v_array[^[[x]]][10]; }", "DeclRefExpr"},
+
// FIXME: the AST has no location info for qualifiers.
{"const [[a^uto]] x = 42;", "AutoTypeLoc"},
{"[[co^nst auto x = 42]];", "VarDecl"},
More information about the cfe-commits
mailing list