sammccall updated this revision to Diff 397062.
sammccall retitled this revision from "[clangd] Fix selection on
multi-dimensional array. (alternate version)" to "[clangd] Fix selection on
multi-dimensional array.".
sammccall edited the summary of this revision.
sammccall added a comment.
cleaner, comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116536/new/
https://reviews.llvm.org/D116536
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -318,6 +318,13 @@
{"[[st^ruct {int x;}]] y;", "CXXRecordDecl"},
{"[[struct {int x;} ^y]];", "VarDecl"},
{"struct {[[int ^x]];} y;", "FieldDecl"},
+
+ // Tricky case: nested ConstantArrayTypeLocs 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"},
+
// FIXME: the AST has no location info for qualifiers.
{"const [[a^uto]] x = 42;", "AutoTypeLoc"},
{"[[co^nst auto x = 42]];", "VarDecl"},
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -58,7 +58,24 @@
SelectionUsedRecovery.record(0, LanguageLabel); // unused.
}
+// Return the range covering a node and all its children.
SourceRange getSourceRange(const DynTypedNode &N) {
+ // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
+ // failing
+ // to descend into the child expression.
+ // decltype(2+2);
+ // ~~~~~~~~~~~~~ <-- correct range
+ // ~~~~~~~~ <-- range reported by getSourceRange()
+ // ~~~~~~~~~~~~ <-- range with this hack(i.e, missing closing paren)
+ // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+ // rid of this patch.
+ if (const auto *TL = N.get<TypeLoc>()) {
+ if (auto DT = TL->getAs<DecltypeTypeLoc>()) {
+ SourceRange S = DT.getSourceRange();
+ S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+ return S;
+ }
+ }
// MemberExprs to implicitly access anonymous fields should not claim any
// tokens for themselves. Given:
// struct A { struct { int b; }; };
@@ -646,17 +663,6 @@
// heuristics. We should consider only pruning critical TypeLoc nodes, to
// be more robust.
- // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
- // failing
- // to descend into the child expression.
- // decltype(2+2);
- // ~~~~~~~~~~~~~ <-- correct range
- // ~~~~~~~~ <-- range reported by getSourceRange()
- // ~~~~~~~~~~~~ <-- range with this hack(i.e, missing closing paren)
- // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
- // rid of this patch.
- if (auto DT = TL->getAs<DecltypeTypeLoc>())
- S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
// AttributedTypeLoc may point to the attribute's range, NOT the modified
// type's range.
if (auto AT = TL->getAs<AttributedTypeLoc>())
@@ -702,7 +708,7 @@
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 +750,28 @@
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];
+ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ArrayType (Outer)
+ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |-ArrayType (Inner)
+ // ~~~~~~~ | |-RecordType
+ // ~~~~~~~~~ | `-Expr
+ // ~~~~~~~~~ `-Expr
+ // Inner 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
