[PATCH] D71644: [clangd] Heuristically resolve dependent call through smart pointer type

[Sam McCall via Phabricator via cfe-commits]

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