ilya-biryukov added inline comments.

================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:846
+             // the underlying FunctionDecl. we should report just one.
+             void $12^$13^F() {}
+           )cpp",
----------------
Could we avoid adding this test in this revision?
We could add it in a follow-up patch along with the fix.


================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:850
+           "1: targets = {Foo::Foo}, decl\n"
+           "2: targets = {Foo::~Foo}, decl\n"
+           "3: targets = {Foo}\n"
----------------
Could we avoid reporting destructor references for now? I feel they would 
create more confusion than bring value.
Mostly worried that they overlap with a reference to the type name and now all 
client code will probably want to filter out destructor references.

I believe they're not critical to any of the use-cases we have so far, having a 
``// FIXME: decide how to surface destructors when we need them` should be good 
enough for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68977



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to