[PATCH] D82739: [clangd] Improve heuristic resolution of dependent types in TargetFinder

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:214
   }
+  if (const auto *CE = dyn_cast<CallExpr>(E)) {
+    const auto *CalleeType = resolveDependentExprToType(CE->getCallee());
----------------
hokein wrote:
> btw, could you try the case below? it doesn't seem to work.
> 
> ```
> struct Bar {
>   int aaaa;
> };
> template <typename T> struct Foo {
>   Bar func(int);
>   void test() {
>     func(1).aa^aa;
>   }
> };
> ```
Thanks for the additional example. To make this work I had to handle one more case (`MemberExpr`) in `resolveDependentExprToDecls()`.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:287
 //
 // FIXME: improve common dependent scope using name lookup in primary templates.
 // e.g. template<typename T> int foo() { return std::vector<T>().size(); }
----------------
hokein wrote:
> this FIXME looks stale now, could you please update it?
I think the FIXME is still relevant. While we handle `DependentScopeDeclRefExpr` and `CXXDependentScopeMemberExpr`, we still do not handle the AST node types listed in the comment.

For example, we do not handle:

```
template <typename>
struct A {
  typedef int [[type]];
};

template <typename T>
struct B {
  typedef typename A<T>::t^ype type;
};
```

(I believe that one would be `DependentNameType`? You can imagine similar scenarios involving using-decls etc.)

However, the example in the FIXME (which depicts a `CXXDependentScopeMemberExpr`) is now stale, so I've removed it and clarified the comment a bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82739/new/

https://reviews.llvm.org/D82739