nridge marked 3 inline comments as done.
nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:139
+// that constructor. FIXME(nridge): this should probably be handled in
+// targetDecl() itself.
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {
----------------
I would appreciate some tips on how we could handle this in `targetDecl()`.
Please see more specific comments below.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:141
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {
+ if (const VarDecl *VD = N->ASTNode.get<VarDecl>()) {
+ if (const Expr *Init = VD->getInit()) {
----------------
This part is intended to handle case #8 in `LocateSymbol.Ambiguous`, where you
have a declaration of the form `Foo abcd("asdf");` which invokes a constructor.
A naive attempt to add this logic to `TargetFinder::add(const Decl*)` resulted
in the constructor being a target even for //references// to the variable, not
just its declaration.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:147
}
+ while ((N = N->Parent)) {
+ if (N->ASTNode.get<CallExpr>() || N->ASTNode.get<ImplicitCastExpr>() ||
----------------
This part is a super hacky way to keep case #9 in `LocateSymbol.Ambiguous`
passing while not regressing other tests.
In that case, the selection tree looks like this:
```
DeclStmt Foo foox = Foo("asdf");
VarDecl Foo foox = Foo("asdf")
ExprWithCleanups Foo("asdf")
CXXConstructExpr Foo("asdf")
MaterializeTemporaryExpr Foo("asdf")
CXXFunctionalCastExpr Foo("asdf")
.RecordTypeLoc Foo
```
The common ancestor is the `RecordTypeLoc`, but we'd like to find the
constructor referred to by the `CXXConstructExpr` as a target as well. To
achieve that, my current code walks up the parent tree until we encounter a
`CXXConstructExpr`, but aborts the walk if certain node types are encountered
to avoid certain other scenarios where we have a `CXXConstructExpr` ancestor.
This is almost certainly wrong, as there are likely many other cases where we
have a `CXXConstructExpr` ancestor but don't want to find the constructor as a
target which are not being handled correctly.
Is there a better way to identify the syntactic form at issue in this testcase?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69237/new/
https://reviews.llvm.org/D69237
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
