[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:379
     if (const TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(D)) {
       add(TND->getUnderlyingType(), Flags | Rel::Underlying);
       Flags |= Rel::Alias; // continue with the alias.
----------------
kadircet wrote:
> rather than introducing a new member for `CurrentlyProcessing`, can't we just get away with checking the decl we are going to jump on here (and probably in namespace alias case) are the same (or maybe a parameter to add function indicating the parent/previous)?
> 
> it is definitely cleaner to have a member rather than checking individually but we have other types of ast nodes we can be currently processing (statements, nested namespecifiers etc. and they are not covered by this member. so it is a little bit confusing conceptually.
It takes a few steps to get back the recursive declaration:

 * we start with the original declaration `D`
 * we cast it to `TypedefNameDecl` here, call `getUnderlyingType()` and call `add(QualType)`
 * in `add(QualType)`, we construct a `TypeVisitor` and call `Visit()`
 * the visitor gets into `VisitDependentNameType()`, calls `getMembersReferencedViaDependentName()`, and that gets us back the same declaration `D` which we call `add(Decl)` on

So, we'd have to propagate the "previous decl" into `add(QualType)` and make it a member of the `TypeVisitor` at least, to be able to check there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382