Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Speculative fix for an infinite recursion with no testcase. I'm happy to circle back to this if/when we get a testcase, but I don't think there's any downside to this patch in the meantime.
Based on the stacktrace, I came up with this crashing example, maybe it's a usable testcase?
template <int> class X { template <int> class Y { using Y<0>::xxx; }; };
(not currently on a machine that can build clangd, so I haven't verified it's the same crash)
Another interesting indirect case
template <int> class X { template <int> class Z; template <int> class Y { using Z<0>::xxx; }; template <int> class Z { using Y<0>::xxx; }; };
clang-tools-extra/clangd/SemanticHighlighting.cpp | ||
---|---|---|
143 | This looks good+safe. I wonder if the check should be moved inside resolveUsingValueDecl - having it resolve to itself seems unhelpful always. |
Thanks for the testcases!
The indirect one is particularly interesting, and suggests that we may want a "seen decls" set (similar to SeenTemplates here) in the kindForDecl() recursion?
(I'd move the check into resolveUsingValueDecl(), but that goes against the grain of putting the more thorough check in the caller -- or should we do both?)
(I'm going to re-request review since I have an outstanding question about the fix approach.)