[COMMIT seastar master] sstring: add == comparison operators

0 views
Skip to first unread message

Commit Bot

<bot@cloudius-systems.com>
unread,
May 22, 2023, 5:29:36 AM5/22/23
to seastar-dev@googlegroups.com, Kefu Chai
From: Kefu Chai <kefu...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>
Branch: master

sstring: add == comparison operators

before this change, we only implemented the <=> operator with
`auto` as its template parameter as a member of sstring class.
and the == and != operator with basic_sstring as members of
sstring class. but this is not enough for some of our use cases,
where we also want to compare sstring with, for instance,
std::string and plain C strings, just like how we compare
an instance of std::string with std::string and plain C strings.
these are legitimate use cases, as we expect sstring as
a drop-in replacement of std::string under most circumstances.

because, even in C++20, the existing operator==(const basic_sstring&)
fails to be selected as a rewrite candidate when compiler tries to
compile a comparison expression like: "a" == sstring("a"), where
the LHS is a plain C string, because the operator!= prevents it to
do so. without the operator!=, the compiler could have synthesized
a candidate of operator==. see https://eel.is/c++draft/over.match.oper#4

the related paragraph of the draft is quoted below:

> A non-template function or function template F named operator==
> is a rewrite target with first operand o unless a search for the
> name operator!= in the scope S from the instantiation context of
> the operator expression finds a function or function template
> that would correspond ([basic.scope.scope]) to F if its name were
> operator==, where S is the scope of the class type of o if F is a
> class member, and the namespace scope of which F is a member
> otherwise.

so, one solution is to simply drop the `operator!=(const basic_sstring&)`.
that actually gets the tests added in this change compile. but the
synthesized operator performs the implicit conversion by calling
the constructor of sstring, which performs a deep copy of another
operand in case whose type is not sstring.

as yet another alternative, we could define an (inline) friend of
operator== and operator!=, they would allow implicit conversion
to sstring if one operator is not an sstring. but again, this hurts
the performance because of the deep copy.

so, in this change, to address the needs from the functionality
and performance perspectives, we add the == operators for
std::basic_string<char_type> and const char*. the new overloads
are selected based on their counterparts of std::string, where
std::string has

* operator==(std::basic_string<char_type...>&) const
* operator==(const char*) const

so we are adding the same set of == operators for sstring.

this should fulfill the needs of the use case where == and
!= operators are used with C++20. it is worth nothing that,
this change does not address the use cases in C++17, where
the LHS of the == comparison expression is not sstring. but
since this change should also benefit the applications in
C++17, the new operators are not guarded by any C++ language
or C++ library feature testing macro guards.

please note, we continue using template to implement the operators,
in order to match with the behavior of std::string. this disallows the
implicit conversions from other types to the operands of the comparsion
operator.

a test is added accordingly (courtesy of Avi).

Signed-off-by: Kefu Chai <kefu...@scylladb.com>

---
diff --git a/include/seastar/core/sstring.hh b/include/seastar/core/sstring.hh
--- a/include/seastar/core/sstring.hh
+++ b/include/seastar/core/sstring.hh
@@ -577,6 +577,12 @@ public:
bool operator!=(const basic_sstring& x) const noexcept {
return !operator==(x);
}
+ constexpr bool operator==(const std::basic_string<char_type> x) const noexcept {
+ return compare(x) == 0;
+ }
+ constexpr bool operator==(const char_type* x) const noexcept {
+ return compare(x) == 0;
+ }
#if __cpp_lib_three_way_comparison
constexpr std::strong_ordering operator<=>(const auto& x) const noexcept {
return compare(x) <=> 0;
diff --git a/tests/unit/sstring_test.cc b/tests/unit/sstring_test.cc
--- a/tests/unit/sstring_test.cc
+++ b/tests/unit/sstring_test.cc
@@ -238,3 +238,18 @@ BOOST_AUTO_TEST_CASE(test_resize_and_overwrite) {
BOOST_CHECK_EQUAL(s, sstring(smaller_size, pattern));
}
}
+
+
+BOOST_AUTO_TEST_CASE(test_compares_left_hand_not_string) {
+ // mostly a compile test for non-sstring left-hand-side
+#if __cplusplus > 201703L
+ BOOST_REQUIRE("a" == sstring("a"));
+ BOOST_REQUIRE(std::string("a") == sstring("a"));
+ BOOST_REQUIRE(std::string_view("a") == sstring("a"));
+#endif
+
+#ifdef __cpp_lib_three_way_comparison
+ BOOST_REQUIRE("a" < sstring("b"));
+ BOOST_REQUIRE(std::string("a") < sstring("b"));
+#endif
+}
Reply all
Reply to author
Forward
0 new messages