[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
This revision was automatically updated to reflect the committed changes.
Closed by commit rG512aa8485010: [clangd] Handle members of anon structs in
SelectionTree (authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"),
Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"),
Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure implicit access of anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure implicit access of anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet updated this revision to Diff 376445.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- update comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"),
Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"),
Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure implicit access of anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure implicit access of anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks!
Comment at: clang-tools-extra/clangd/Selection.cpp:446
return true;
+ // Make sure anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
hmm, "implicit accesses of anonymous structs"?
Comment at: clang-tools-extra/clangd/Selection.cpp:450
+ if (FD->isAnonymousStructOrUnion())
+// If the base is not implicit, it can own tokens, otherwise we should
+// stop traversal.
The double negatives and references to a "traversal" here seem confusing.
Maybe something more concrete?
```
// If Base is an implicit CXXThis, then the whole MemberExpr has no tokens.
// If it's a normal e.g. DeclRef, we treat the MemberExpr like an implicit cast.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:443 // It would be nice if RAV handled this (!shouldTraverseImplicitCode()). if (auto *CTI = llvm::dyn_cast(S)) if (CTI->isImplicit()) sammccall wrote: > kadircet wrote: > > sammccall wrote: > > > seems like it'd be more consistent to handle it here? > > > > > > If it's a MemberExpr and the member is in an anonymous struct, then it's > > > implicit if `isImplicit(ME.getBase())`. > > the test case was actually to demonstrate why we can't do it here. we want > > to still keep traversing the AST after hitting a field inside an anon > > struct. handling here terminates the traversal for that subtree completely, > > and the real node we are interested might be down the tree (`y.[[x]]` in > > the test case). > That's what I meant by `isImplicit(ME.getBase())` > > The following passes tests (added to isImplicit)... > ``` > if (auto *ME = llvm::dyn_cast(S)) > if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) > if (FD->isAnonymousStructOrUnion()) > return isImplicit(ME->getBase()); > ``` > > > and the member is in an anonymous struct > > Oops, I meant "and the member *is* an anonymous struct". > It seems more natural to recognize and special-case the field-access to the > anon struct itself (which is intuitively implicit!), than the one to a normal > field within it. right, it makes sense. i was missing the isImplicit(base) bit + got stunned by the beautiful shape of the AST. thanks for bearing with me :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110825/new/ https://reviews.llvm.org/D110825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet updated this revision to Diff 376238.
kadircet marked 4 inline comments as done.
kadircet added a comment.
- Use `isImplicit` rather than `earlyClaim`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"),
Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"),
Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,14 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If the base is not implicit, it can own tokens, otherwise we should
+// stop traversal.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+ struct { int $1[[x]]; };
+ void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+ }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+ EXPECT_THAT(
+ locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+ UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,14 @@
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
return true;
+ // Make sure anonymous structs don't end up owning tokens.
+ if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+ if (FD->isAnonymousStructOrUnion())
+// If the base is not implicit, it can own tokens, otherwise we should
+// stop traversal.
+return isImplicit(ME->getBase());
+ }
// Refs to operator() and [] are (almost?) always implicit as part of calls.
if (auto *DRE = llvm::dyn_cast(S)) {
if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:368
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
kadircet wrote:
> sammccall wrote:
> > This example is quite complicated to the point where it's hard to see what
> > it's testing... The example you have in the comment is much clearer.
> >
> > If we also want to test named fields with inline struct types, maybe that
> > should be a second test...
> it is actually important to demonstrate we are not claiming the whole range
> at the first encounter (and to make sure traversal is not stopped for any
> other reason).
Makes sense, but without comment it's not clear.
Can we make the latter a separate test case, and simplify it too? This seems to
be enough:
```
struct [[Foo]] {
struct { int x; }
};
int a = ^Foo{}.x;
```
Here the Foo{} is the non-empty base of the anon member access that must be
traversed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
sammccall added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:443 // It would be nice if RAV handled this (!shouldTraverseImplicitCode()). if (auto *CTI = llvm::dyn_cast(S)) if (CTI->isImplicit()) kadircet wrote: > sammccall wrote: > > seems like it'd be more consistent to handle it here? > > > > If it's a MemberExpr and the member is in an anonymous struct, then it's > > implicit if `isImplicit(ME.getBase())`. > the test case was actually to demonstrate why we can't do it here. we want to > still keep traversing the AST after hitting a field inside an anon struct. > handling here terminates the traversal for that subtree completely, and the > real node we are interested might be down the tree (`y.[[x]]` in the test > case). That's what I meant by `isImplicit(ME.getBase())` The following passes tests (added to isImplicit)... ``` if (auto *ME = llvm::dyn_cast(S)) if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) if (FD->isAnonymousStructOrUnion()) return isImplicit(ME->getBase()); ``` > and the member is in an anonymous struct Oops, I meant "and the member *is* an anonymous struct". It seems more natural to recognize and special-case the field-access to the anon struct itself (which is intuitively implicit!), than the one to a normal field within it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110825/new/ https://reviews.llvm.org/D110825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Selection.cpp:443
// It would be nice if RAV handled this (!shouldTraverseImplicitCode()).
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
sammccall wrote:
> seems like it'd be more consistent to handle it here?
>
> If it's a MemberExpr and the member is in an anonymous struct, then it's
> implicit if `isImplicit(ME.getBase())`.
the test case was actually to demonstrate why we can't do it here. we want to
still keep traversing the AST after hitting a field inside an anon struct.
handling here terminates the traversal for that subtree completely, and the
real node we are interested might be down the tree (`y.[[x]]` in the test case).
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:368
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
sammccall wrote:
> This example is quite complicated to the point where it's hard to see what
> it's testing... The example you have in the comment is much clearer.
>
> If we also want to test named fields with inline struct types, maybe that
> should be a second test...
it is actually important to demonstrate we are not claiming the whole range at
the first encounter (and to make sure traversal is not stopped for any other
reason).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
sammccall added a comment.
Thanks for fixing!
Early-claim is a blunt hammer and I'm always dreading side-effects.
This also feels a *lot* like implicit-this which is handled in another way.
So that seems worth a shot, but this approach is fine if that one doesn't work.
Comment at: clang-tools-extra/clangd/Selection.cpp:443
// It would be nice if RAV handled this (!shouldTraverseImplicitCode()).
if (auto *CTI = llvm::dyn_cast(S))
if (CTI->isImplicit())
seems like it'd be more consistent to handle it here?
If it's a MemberExpr and the member is in an anonymous struct, then it's
implicit if `isImplicit(ME.getBase())`.
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:368
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
This example is quite complicated to the point where it's hard to see what it's
testing... The example you have in the comment is much clearer.
If we also want to test named fields with inline struct types, maybe that
should be a second test...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Selection.cpp:745
+ if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) {
+if (FD->getParent()->isAnonymousStructOrUnion())
+ return SourceRange(ME->getMemberLoc(), ME->getEndLoc());
in theory this check should be redundant, as each field should own its tokens.
but given the complexity of C++ just wanted to be conservative.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110825/new/
https://reviews.llvm.org/D110825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
References to fields inside anon structs contain an implicit children
for the container, which has the same SourceLocation with the field.
This was resulting in SelectionTree always picking the anon-struct rather than
the field as the selection.
This patch prevents that by claiming the range for the field early.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D110825
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,25 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"),
Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+class Foo {
+ struct {
+struct {
+ int $1[[x]];
+} $0[[y]];
+ };
+ void foo() { $0^y.$1^x = 2; }
+};
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("0"), TU.index().get()),
+ UnorderedElementsAre(Sym("y", Code.range("0"),
Code.range("0";
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"),
Code.range("1";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -732,6 +732,19 @@
} else if (const auto *CCI = N.get()) {
// : [[b_]](42)
return CCI->getMemberLocation();
+} else if (const auto *ME = N.get()) {
+ // In member expressions involving anonymous structs, there's an implicit
+ // reference to the container. Make sure all the tokens that belong to
the
+ // member are claimed first to prevent this unspelled children from
owning
+ // them.
+ // struct Foo {
+ // struct { int x; }
+ // void foo() { [[x]] = 2; }
+ // };
+ if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) {
+if (FD->getParent()->isAnonymousStructOrUnion())
+ return SourceRange(ME->getMemberLoc(), ME->getEndLoc());
+ }
}
return SourceRange();
}
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,25 @@
ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
}
+TEST(LocateSymbol, AnonymousStructFields) {
+ auto Code = Annotations(R"cpp(
+class Foo {
+ struct {
+struct {
+ int $1[[x]];
+} $0[[y]];
+ };
+ void foo() { $0^y.$1^x = 2; }
+};
+ )cpp");
+ TestTU TU = TestTU::withCode(Code.code());
+ auto AST = TU.build();
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("0"), TU.index().get()),
+ UnorderedElementsAre(Sym("y", Code.range("0"), Code.range("0";
+ EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+ UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+}
+
TEST(LocateSymbol, FindOverrides) {
auto Code = Annotations(R"cpp(
class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -732,6 +732,19 @@
} else if (const auto *CCI = N.get()) {
// : [[b_]](42)
return CCI->getMemberLocation();
+} else if (const auto *ME = N.get()) {
+ // In member expressions involving anonymous structs, there's an implicit
+ // reference to the container. Make sure all the tokens that belong to the
+ // member are claimed first to prevent this unspelled children from owning
+ // them.
+ // struct Foo {
+ // struct { int x; }
+ // void foo() { [[x]] = 2; }
+ // };
+ if (auto *FD = llvm::dyn_cast(ME->getMemberDecl())) {
+if (FD->getParent()->isAnonymousStructOrUnion())
+ return SourceRange(ME->getMemberLoc(), ME->getEndLoc());
+ }
}
return SourceRange();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
