Skip to content

Commit bb41f85

Browse files
committed
[clangd] Correct SelectionTree behavior around anonymous field access.
struct A { struct { int b; }; }; A().^b; This should be considered a reference to b, but currently it's considered a reference to the anonymous struct field. Fixes clangd/clangd#798 Differential Revision: https://reviews.llvm.org/D104376
1 parent 14d8f15 commit bb41f85

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

clang-tools-extra/clangd/Selection.cpp

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,27 @@ void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
5757
SelectionUsedRecovery.record(0, LanguageLabel); // unused.
5858
}
5959

60+
SourceRange getSourceRange(const DynTypedNode &N) {
61+
// MemberExprs to implicitly access anonymous fields should not claim any
62+
// tokens for themselves. Given:
63+
// struct A { struct { int b; }; };
64+
// The clang AST reports the following nodes for an access to b:
65+
// A().b;
66+
// [----] MemberExpr, base = A().<anonymous>, member = b
67+
// [----] MemberExpr: base = A(), member = <anonymous>
68+
// [-] CXXConstructExpr
69+
// For our purposes, we don't want the second MemberExpr to own any tokens,
70+
// so we reduce its range to match the CXXConstructExpr.
71+
// (It's not clear that changing the clang AST would be correct in general).
72+
if (const auto *ME = N.get<MemberExpr>()) {
73+
if (!ME->getMemberDecl()->getDeclName())
74+
return ME->getBase()
75+
? getSourceRange(DynTypedNode::create(*ME->getBase()))
76+
: SourceRange();
77+
}
78+
return N.getSourceRange();
79+
}
80+
6081
// An IntervalSet maintains a set of disjoint subranges of an array.
6182
//
6283
// Initially, it contains the entire array.
@@ -608,7 +629,7 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
608629
// An optimization for a common case: nodes outside macro expansions that
609630
// don't intersect the selection may be recursively skipped.
610631
bool canSafelySkipNode(const DynTypedNode &N) {
611-
SourceRange S = N.getSourceRange();
632+
SourceRange S = getSourceRange(N);
612633
if (auto *TL = N.get<TypeLoc>()) {
613634
// FIXME: TypeLoc::getBeginLoc()/getEndLoc() are pretty fragile
614635
// heuristics. We should consider only pruning critical TypeLoc nodes, to
@@ -665,7 +686,7 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
665686
void pop() {
666687
Node &N = *Stack.top();
667688
dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
668-
claimRange(N.ASTNode.getSourceRange(), N.Selected);
689+
claimRange(getSourceRange(N.ASTNode), N.Selected);
669690
if (N.Selected == NoTokens)
670691
N.Selected = SelectionTree::Unselected;
671692
if (N.Selected || !N.Children.empty()) {
@@ -868,13 +889,13 @@ const DeclContext &SelectionTree::Node::getDeclContext() const {
868889

869890
const SelectionTree::Node &SelectionTree::Node::ignoreImplicit() const {
870891
if (Children.size() == 1 &&
871-
Children.front()->ASTNode.getSourceRange() == ASTNode.getSourceRange())
892+
getSourceRange(Children.front()->ASTNode) == getSourceRange(ASTNode))
872893
return Children.front()->ignoreImplicit();
873894
return *this;
874895
}
875896

876897
const SelectionTree::Node &SelectionTree::Node::outerImplicit() const {
877-
if (Parent && Parent->ASTNode.getSourceRange() == ASTNode.getSourceRange())
898+
if (Parent && getSourceRange(Parent->ASTNode) == getSourceRange(ASTNode))
878899
return Parent->outerImplicit();
879900
return *this;
880901
}

clang-tools-extra/clangd/unittests/XRefsTests.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,18 @@ TEST(LocateSymbol, All) {
420420
// $def is the definition location (if absent, symbol has no definition)
421421
// unnamed range becomes both $decl and $def.
422422
const char *Tests[] = {
423+
R"cpp(
424+
struct X {
425+
union {
426+
int [[a]];
427+
float b;
428+
};
429+
};
430+
int test(X &x) {
431+
return x.^a;
432+
}
433+
)cpp",
434+
423435
R"cpp(// Local variable
424436
int main() {
425437
int [[bonjour]];

0 commit comments

Comments
 (0)