[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this looks pretty good!

> I'm not sure if I the hover test which is added in this patch is the right one,
> but at least is passed with this patch and fails without it :)

This is nice to have, but you add a unittest to FindTargetTest too? That's the most direct way to test this code.

This won't affect find-refs BTW, that would be handled in `refInStmt()` in FindTarget.cpp



================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
 
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+  Annotations T(R"cpp(
----------------
no need to sign your work :-)


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
 
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+  Annotations T(R"cpp(
----------------
sammccall wrote:
> no need to sign your work :-)
can you add this to HoverTest__All instead? That way we test all details of the hover card


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4079
+  EXPECT_EQ(HI->Type,
+            HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+  EXPECT_EQ(HI->Documentation, "Foo spaceship");
----------------
if we're describing this as the spaceship operator, then the type looks wrong


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153331