[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f065bfd633d: [clangd][c++20]Consider rewritten binary
operators in TargetFinder (authored by massberg).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2927,6 +2927,41 @@
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 @@
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 @@
EXPECT_TRUE(H->Value);
EXPECT_TRUE(H->Type);
}
-
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -618,6 +618,36 @@
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.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg updated this revision to Diff 533211.
massberg marked an inline comment as done.
massberg added a comment.
clang-format code
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2926,6 +2926,41 @@
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.
@@ -2941,7 +2976,7 @@
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");
@@ -4047,7 +4082,6 @@
EXPECT_TRUE(H->Value);
EXPECT_TRUE(H->Type);
}
-
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -618,6 +618,36 @@
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.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg marked an inline comment as done.
massberg added inline comments.
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(R"cpp(
sammccall wrote:
> massberg wrote:
> > massberg wrote:
> > > sammccall wrote:
> > > > 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
> > > Upps, sorry.
> > > can you add this to HoverTest__All instead? That way we test all details
> > > of the hover card
> >
> > The (Hover, All) test tests with `std=c++17` while this test tests c++20
> > features.
> > We could add an additional field with the version to the struct in the
> > (Hover, All) test.
> > Or add a (Hover, All_Cpp20) test for testing C++20 (what is probably not
> > worth at the moment with just one test requiring C++20).
> Does anything break if you switch everything to C++20?
> The intention of "std=c++17" there was certainly "not 14", rather than "not
> 20" :-)
Test test passed with C++20. I have switch it to C++20 and moved the new test
as a new case in this test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg updated this revision to Diff 533204.
massberg marked 2 inline comments as done.
massberg added a comment.
Switched the "Hover, All" test to run with C++20 instead of C++17 and moved the
RewrittenBinaryOperatorSpaceship test to it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2926,6 +2926,41 @@
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.
@@ -2941,7 +2976,7 @@
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");
@@ -4047,7 +4082,6 @@
EXPECT_TRUE(H->Value);
EXPECT_TRUE(H->Type);
}
-
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -618,6 +618,36 @@
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.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:648
+ {"bool operator==(const Foo &) const noexcept = default",
+Rel::TemplateInstantiation});
+}
this template pattern vs instantiation is really surprising, but it's a
reasonable analogy, I don't see any particular problems. (Also, `operator<=>`
looks like a template over whatever `=` is!)
Do you know whether this means go-to-definition gives you two options to
navigate to?
(No need to write a test, just curious)
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(R"cpp(
massberg wrote:
> massberg wrote:
> > sammccall wrote:
> > > 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
> > Upps, sorry.
> > can you add this to HoverTest__All instead? That way we test all details of
> > the hover card
>
> The (Hover, All) test tests with `std=c++17` while this test tests c++20
> features.
> We could add an additional field with the version to the struct in the
> (Hover, All) test.
> Or add a (Hover, All_Cpp20) test for testing C++20 (what is probably not
> worth at the moment with just one test requiring C++20).
Does anything break if you switch everything to C++20?
The intention of "std=c++17" there was certainly "not 14", rather than "not 20"
:-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg marked 2 inline comments as done.
massberg added a comment.
Thanks for the comments, I have added an additional test to FindTargetTest. See
also my other comments.
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(R"cpp(
sammccall wrote:
> 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
Upps, sorry.
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(R"cpp(
massberg wrote:
> sammccall wrote:
> > 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
> Upps, sorry.
> can you add this to HoverTest__All instead? That way we test all details of
> the hover card
The (Hover, All) test tests with `std=c++17` while this test tests c++20
features.
We could add an additional field with the version to the struct in the (Hover,
All) test.
Or add a (Hover, All_Cpp20) test for testing C++20 (what is probably not worth
at the moment with just one test requiring C++20).
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");
sammccall wrote:
> if we're describing this as the spaceship operator, then the type looks wrong
I have added a test checking the whole definition.
Actually the following is happening here:
The `!=` operator isn't explicitly defined, so the binary operator is rewritten
to `!(Foo(1) == Foo(2)`,
i.e. we are now using the `==` operator.
However, the `==` operator is also not explicitly defined, but there is a
defaulted spaceship operator.
Thus the `==` operator is implicitly defined through the `<=>` operator.
So the type and definition here are from the implicitly defined `==` operator,
while the original source of it is the `<=>` operator.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg updated this revision to Diff 532921.
massberg added a comment.
Add test to FindTargetTests and extend test in HoverTests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4048,6 +4048,41 @@
EXPECT_TRUE(H->Type);
}
+TEST(Hover, RewrittenBinaryOperatorSpaceship) {
+ Annotations T(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;
+ };
+
+ static_assert(Foo(1) !^= Foo(2));
+ )cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Type,
+HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+ EXPECT_EQ(HI->Name, "operator==");
+ EXPECT_EQ(HI->Definition,
+"bool operator==(const Foo &) const noexcept = default");
+ EXPECT_EQ(HI->Documentation, "Foo spaceship");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -618,6 +618,36 @@
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.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg updated this revision to Diff 532867.
massberg added a comment.
Fix test name
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153331/new/
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4048,6 +4048,38 @@
EXPECT_TRUE(H->Type);
}
+TEST(Hover, RewrittenBinaryOperatorSpaceship) {
+ Annotations T(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;
+ };
+
+ static_assert(Foo(1) !^= Foo(2));
+ )cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Type,
+HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+ EXPECT_EQ(HI->Documentation, "Foo spaceship");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4048,6 +4048,38 @@
EXPECT_TRUE(H->Type);
}
+TEST(Hover, RewrittenBinaryOperatorSpaceship) {
+ Annotations T(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;
+ };
+
+ static_assert(Foo(1) !^= Foo(2));
+ )cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Type,
+HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+ EXPECT_EQ(HI->Documentation, "Foo spaceship");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder
massberg created this revision.
massberg added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
massberg requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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==".
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153331
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4048,6 +4048,38 @@
EXPECT_TRUE(H->Type);
}
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(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;
+ };
+
+ static_assert(Foo(1) !^= Foo(2));
+ )cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Type,
+HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+ EXPECT_EQ(HI->Documentation, "Foo spaceship");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
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);
}
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4048,6 +4048,38 @@
EXPECT_TRUE(H->Type);
}
+TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) {
+ Annotations T(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;
+ };
+
+ static_assert(Foo(1) !^= Foo(2));
+ )cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Type,
+HoverInfo::PrintedType("bool (const Foo &) const noexcept"));
+ EXPECT_EQ(HI->Documentation, "Foo spaceship");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -347,6 +347,10 @@
void VisitCXXDeleteExpr(const CXXDeleteExpr *CDE) {
Outer.add(CDE->getOperatorDelete(), Flags);
}
+ void
+ VisitCXXRewrittenBinaryOperator(con
