[PATCH] D76103: [clangd] Extend findTarget()'s dependent name heuristic to handle enumerators

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! Just a couple of Qs...



================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:105
+  if (auto *ET = T->getAs<EnumType>()) {
+    LookupInDecl = ET->getDecl();
+  } else {
----------------
Is this the bit that "breaks" the dependence? getDecl() works even if the type might ultimately correspond to a different decl?

It's not obvious to me where the magic happens, so wherever it is, maybe add a comment :-)


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:105
+  if (auto *ET = T->getAs<EnumType>()) {
+    LookupInDecl = ET->getDecl();
+  } else {
----------------
sammccall wrote:
> Is this the bit that "breaks" the dependence? getDecl() works even if the type might ultimately correspond to a different decl?
> 
> It's not obvious to me where the magic happens, so wherever it is, maybe add a comment :-)
The actual lookup doesn't do anything subtle in this case (no dependent base lookup, failing to match IsArrow isn't possible).

What do you think about just calling Decl->lookup() here and early returning, not bothering with the generalization of lookupDependentName? (I find the mix of behavior in that function a little hard to get my head around)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76103