sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This is great! Thank you!
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:116
+ // smart pointer type.
+ ASTContext *Ctx = hackyFindASTContext(T);
+ if (!Ctx)
----------------
nridge wrote:
> I don't intend for this function to be in the final patch.
>
> Rather, I'm wondering: am I missing some obvious general way to recover the
> `ASTContext` from a `Type` (or a `Stmt`)?
>
> If not, I think we'll need to modify `targetDecl()`, `allTargetDecls()`,
> `findExplicitReferences()` etc. to have their callers pass in an `ASTContext`?
Haha, I see what you mean. There isn't a general way, but I think your hack is
better than changing the public API, and we can improve it a little.
By the time you actually do the lookup, you have a CXXRecordDecl you're going
to look into, and decls have a pointer to the ASTContext. So instead of a
DeclarationName, you could pass a function_ref<const
DeclarationName&(ASTContext&)> or such into
`getMembersReferencedViaDependentName`, and have that function lazily call the
factory to get the DeclarationName to use. Still hacky, but it's an internal
function, and we don't have the weird parallel codepath...
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:503
R"cpp(// FIXME: Heuristic resolution of dependent method
// invoked via smart pointer
----------------
you can remove the FIXME
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71644/new/
https://reviews.llvm.org/D71644
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
