sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:90
+ for (const auto &Entry :
+ allTargetDecls(N->ASTNode, AST.getHeuristicResolver()))
ActualDecls.emplace_back(Entry.first, Entry.second);
----------------
nridge wrote:
> sammccall wrote:
> > If we really want these to be separate, I think we should be using a null
> > resolver except for the tests that are explicitly testing heuristics
> > resolution. (And those tests could check both with/without resolver and
> > verify it's required...)
> I'm not sure what you mean by "if we really want these to be separate". What
> are "these"?
I mean the separation between the "hard" AST logic in FindTarget and the
heuristic resolution. And by "we" I really mean "I", I guess :-)
Ideally we'd test these both separately and smoke-test the interaction, but I
think the low-hanging fruit is to use all the existing tests, with most of them
testing FindTarget in isolation and the few exceptions testing
FindTarget+HeuristicResolver together.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92290/new/
https://reviews.llvm.org/D92290
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
