From 6f065bfd633d7ca006f62b894108d5369dc46836 Mon Sep 17 00:00:00 2001
From: Jens Massberg <massberg@google.com>
Date: Tue, 20 Jun 2023 13:25:56 +0200
Subject: [PATCH] [clangd][c++20]Consider rewritten binary operators in
 TargetFinder

In C++20 some binary operations can be rewritten, e.g. `a != b`
can be rewritten to `!(a == b)` if `!=` is not explicitly defined.
The `TargetFinder` hasn't considered the corresponding `CXXRewrittenBinaryOperator` yet. This resulted that the definition of such operators couldn't be found
when navigating to such a `!=` operator, see https://github.com/clangd/clangd/issues/1476.

In this patch we add support of `CXXRewrittenBinaryOperator` in `FindTarget`.
In such a case we redirect to the inner binary operator of the decomposed form.
E.g. in case that `a != b` has been rewritten to `!(a == b)` we go to the
`==` operator. The `==` operator might be implicitly defined (e.g. by a `<=>`
operator), but this case is already handled, see the new test.

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 :)

Note, that it might be a bit missleading that hovering over a `!=` refers to
"instance method operator==".

Differential Revision: https://reviews.llvm.org/D153331
---
 clang-tools-extra/clangd/FindTarget.cpp            |  4 +++
 .../clangd/unittests/FindTargetTests.cpp           | 30 +++++++++++++++++
 clang-tools-extra/clangd/unittests/HoverTests.cpp  | 38 ++++++++++++++++++++--
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index efbd05e..eead9e6 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@ public:
       void VisitCXXDeleteExpr(const CXXDeleteExpr *CDE) {
         Outer.add(CDE->getOperatorDelete(), Flags);
       }
+      void
+      VisitCXXRewrittenBinaryOperator(const CXXRewrittenBinaryOperator *RBO) {
+        Outer.add(RBO->getDecomposedForm().InnerBinOp, Flags);
+      }
     };
     Visitor(*this, Flags).Visit(S);
   }
diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 81e891b..9979628 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -618,6 +618,36 @@ TEST_F(TargetDeclTest, Coroutine) {
   EXPECT_DECLS("RecordTypeLoc", "struct executor");
 }
 
+TEST_F(TargetDeclTest, RewrittenBinaryOperator) {
+  Flags.push_back("-std=c++20");
+
+  Code = R"cpp(
+  namespace std {
+    struct strong_ordering {
+      int n;
+      constexpr operator int() const { return n; }
+      static const strong_ordering equal, greater, less;
+    };
+    constexpr strong_ordering strong_ordering::equal = {0};
+    constexpr strong_ordering strong_ordering::greater = {1};
+    constexpr strong_ordering strong_ordering::less = {-1};
+    }
+
+    struct Foo
+    {
+      int x;
+      auto operator<=>(const Foo&) const = default;
+    };
+
+    bool x = (Foo(1) [[!=]] Foo(2));
+  )cpp";
+  EXPECT_DECLS("CXXRewrittenBinaryOperator",
+               {"std::strong_ordering operator<=>(const Foo &) const = default",
+                Rel::TemplatePattern},
+               {"bool operator==(const Foo &) const noexcept = default",
+                Rel::TemplateInstantiation});
+}
+
 TEST_F(TargetDeclTest, FunctionTemplate) {
   Code = R"cpp(
     // Implicit specialization.
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 5338a68..7002e27 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2927,6 +2927,41 @@ TEST(Hover, All) {
          HI.Definition = "__attribute__((nonnull))";
          HI.Documentation = Attr::getDocumentation(attr::NonNull).str();
        }},
+      {
+          R"cpp(
+          namespace std {
+          struct strong_ordering {
+            int n;
+            constexpr operator int() const { return n; }
+            static const strong_ordering equal, greater, less;
+          };
+          constexpr strong_ordering strong_ordering::equal = {0};
+          constexpr strong_ordering strong_ordering::greater = {1};
+          constexpr strong_ordering strong_ordering::less = {-1};
+          }
+
+          struct Foo
+          {
+            int x;
+            // Foo spaceship
+            auto operator<=>(const Foo&) const = default;
+          };
+
+          bool x = Foo(1) [[!^=]] Foo(2);
+         )cpp",
+          [](HoverInfo &HI) {
+            HI.Type = "bool (const Foo &) const noexcept";
+            HI.Value = "true";
+            HI.Name = "operator==";
+            HI.Parameters = {{{"const Foo &"}, std::nullopt, std::nullopt}};
+            HI.ReturnType = "bool";
+            HI.Kind = index::SymbolKind::InstanceMethod;
+            HI.LocalScope = "Foo::";
+            HI.NamespaceScope = "";
+            HI.Definition =
+                "bool operator==(const Foo &) const noexcept = default";
+            HI.Documentation = "Foo spaceship";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
@@ -2942,7 +2977,7 @@ TEST(Hover, All) {
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
-    TU.ExtraArgs.push_back("-std=c++17");
+    TU.ExtraArgs.push_back("-std=c++20");
     TU.ExtraArgs.push_back("-xobjective-c++");
 
     TU.ExtraArgs.push_back("-Wno-gnu-designator");
@@ -4076,7 +4111,6 @@ constexpr u64 pow_with_mod(u64 a, u64 b, u64 p) {
   EXPECT_TRUE(H->Value);
   EXPECT_TRUE(H->Type);
 }
-
 } // namespace
 } // namespace clangd
 } // namespace clang
-- 
2.7.4