[clang-tools-extra] [clangd] [HeuristicResolver] Protect against infinite recursion on DependentNameTypes (PR #83542)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 1 23:09:02 PST 2024


https://github.com/zyn0217 approved this pull request.

Thank you for the quick fix! While I think the patch looks generally good to me, it would be better to add some analysis of the bug.

That said, I took a closer look at it, and I think I probably have another similar approach that doesn't involve extensive refactors. And here is my analysis:

(Let's use the example from your test case)

```cpp
template <int N>
struct waldo {
  using type = waldo<N - 1>::type::next;
};
```
So, we're somehow attempting to resolve the type/decl for `next`, which is of `DependentNameType`. To achieve that, we want to inspect the type of its prefix i.e. the `waldo<N - 1>::type`, which is another `NestedNameSpecifier` and thus we shall find out the type of `waldo<N - 1>` first - which is of `ClassTemplateSpecializationType`. Then, we would perform a name lookup for `type` within the context thereof, which results in the solely `TypedefType` we're currently in.

Then we move on and try to find out what the `next` is with the `TypedefType` as its prefix. This would happen in `resolveDependentMember`, with `T` as `TypedefType` and `Name` as `next`. We expect that prefix to be of a `RecordType` so that we can perform a name lookup. That is `resolveTypeToRecordDecl`, and the type alias would get unwrapped and become a `DependentNameType` in the end.

And now we're successfully getting into an infinite loop: we want to know the type of `type`, which ends up a type alias `type` that would boil down to itself!

The PR resolves it by tracking every seen DependentNameType with a set. And since we don't want to turn the stateless `HeuristicResolver` to something stateful (e.g. adding extra data members), we have to add another layer to hide our implementation - this is what you've done.

However, I think we could track these `DependentNameType`s by adding some extra parameters. Specifically, we can pass through the `DNT` from `resolveDependentNameType` to `resolveDependentMember` and `resolveTypeToRecordDecl`, then inside `resolveTypeToRecordDecl`, we bail out with null if the unwrapped `DependentNameType` from `T` is the same as the `DNT` - I did an experiment and it turns out to be working.
 
That said, I'd like to leave the decision up to you - my approach doesn't seem much clearer than yours!

(Please add some details to the commit message before merging :-)

https://github.com/llvm/llvm-project/pull/83542


More information about the cfe-commits mailing list