ilya-biryukov added a comment.
Mostly LGTM, thanks! Just a few clarifying questions and suggestions
This API allows us to get a target declaration for a reference, but does not
help with finding the source locations for those references. Do you plan to put
this into the API? Have this as a separate function?
E.g. finding a source location of `field` for `DeclRefExpr` produced for
`MyBase::field` seems to be the same amount of work (in general case) as
finding the `Decl*` of `field`.
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:374
+ const char *Sep = "";
+ for (unsigned I = 0; I < RS.S.size(); ++I)
+ if (RS.S.test(I)) {
----------------
NIT: maybe add braces around multi-line body of the for statement?
================
Comment at: clang-tools-extra/clangd/FindTarget.h:70
+llvm::SmallVector<const Decl *, 1>
+targetDecl(const ast_type_traits::DynTypedNode &, DeclRelationSet Mask);
+
----------------
Could we add convenience overloads for `Expr*`, `QualType`, `TypeLoc` and maybe
other interesting cases?
Having to call `DynTypedNode::create` for each invocation of these two
functions would feel awkward, saving some typing for the clients seems to be
worth a little boilerplate.
================
Comment at: clang-tools-extra/clangd/FindTarget.h:86
+ /// This declaration is an alias that was referred to.
+ Alias,
+ /// This is the underlying declaration for an alias, decltype etc.
----------------
Could you provide examples for `Alias` and `Underlying` in the comments?
One with a template and one with a namespace alias
================
Comment at: clang-tools-extra/clangd/FindTarget.h:92
+ /// (e.g. a delegating constructor has a pure-lexical reference to the class)
+ PureLexical,
+};
----------------
Could you also provide an example here?
It this a delegating constructor in the constructor-init-list?
================
Comment at: clang-tools-extra/clangd/FindTarget.h:103
+ DeclRelationSet() = default;
+ DeclRelationSet(DeclRelation R) { S.set(static_cast<unsigned long long>(R));
}
+
----------------
Why not `unsigned`? What are the interesting corner cases?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66751/new/
https://reviews.llvm.org/D66751
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
