Repository: dtolnay/cxx (language: rust)

Issue:
Provide comparison operators for rust::Str and rust::String
The C++ types should come with operators for `==`, `!=`, `<`, `>`, `<=`, `>=`.


Discussion / hints:
While looking into adding this, I noticed that a few trivial functions of `alloc::String` are routed via FFI, e.g. `cxxbridge1$string$len` for `String::len`. So long as `Vec` has a guaranteed memory layout, and `String` is a `Vec<u8>`, then it should be possible to get the length out of `rust::String::repr[1]`, and similarly for the pointer.

I bring this up because, if this would work reliably, then all these operators could be implemented with just an equality and a comparison function that operates on 2 string slices (technically two pointer + length pairs, since `&str` is not FFI safe), in addition to removing the accessors mentioned above.

`alloc::String` is not yet marked as `repr_transparent`, pending https://github.com/rust-lang/rust/issues/70473.

Do you think having `repr_transparent` on `String` is really a requirement in practice for this case, or should this go ahead before the issue above is implemented ? Because personally, I can't see how the layout of `String` could differ from `Vec<u8>` in practice, given its implementation.
Vec does not have a guaranteed layout.

Fix patch:
diff --git a/book/src/binding/str.md b/book/src/binding/str.md
index 93e975026..999727c06 100644
--- a/book/src/binding/str.md
+++ b/book/src/binding/str.md
@@ -37,6 +37,13 @@ public:
   const_iterator end() const noexcept;
   const_iterator cbegin() const noexcept;
   const_iterator cend() const noexcept;
+
+  bool operator==(const Str &) const noexcept;
+  bool operator!=(const Str &) const noexcept;
+  bool operator<(const Str &) const noexcept;
+  bool operator<=(const Str &) const noexcept;
+  bool operator>(const Str &) const noexcept;
+  bool operator>=(const Str &) const noexcept;
 };
 
 std::ostream &operator<<(std::ostream &, const Str &);
diff --git a/book/src/binding/string.md b/book/src/binding/string.md
index 47a2b964f..c4a1b6ad4 100644
--- a/book/src/binding/string.md
+++ b/book/src/binding/string.md
@@ -42,6 +42,13 @@ public:
   const_iterator end() const noexcept;
   const_iterator cbegin() const noexcept;
   const_iterator cend() const noexcept;
+
+  bool operator==(const String &) const noexcept;
+  bool operator!=(const String &) const noexcept;
+  bool operator<(const String &) const noexcept;
+  bool operator<=(const String &) const noexcept;
+  bool operator>(const String &) const noexcept;
+  bool operator>=(const String &) const noexcept;
 };
 
 std::ostream &operator<<(std::ostream &, const String &);
diff --git a/include/cxx.h b/include/cxx.h
index e40876ddf..31b34fc78 100644
--- a/include/cxx.h
+++ b/include/cxx.h
@@ -59,6 +59,13 @@ class String final {
   const_iterator cbegin() const noexcept;
   const_iterator cend() const noexcept;
 
+  bool operator==(const String &) const noexcept;
+  bool operator!=(const String &) const noexcept;
+  bool operator<(const String &) const noexcept;
+  bool operator<=(const String &) const noexcept;
+  bool operator>(const String &) const noexcept;
+  bool operator>=(const String &) const noexcept;
+
   // Internal API only intended for the cxxbridge code generator.
   String(unsafe_bitcopy_t, const String &) noexcept;
 
@@ -98,6 +105,13 @@ class Str final {
   const_iterator cbegin() const noexcept;
   const_iterator cend() const noexcept;
 
+  bool operator==(const Str &) const noexcept;
+  bool operator!=(const Str &) const noexcept;
+  bool operator<(const Str &) const noexcept;
+  bool operator<=(const Str &) const noexcept;
+  bool operator>(const Str &) const noexcept;
+  bool operator>=(const Str &) const noexcept;
+
 private:
   friend impl<Str>;
   // Not necessarily ABI compatible with &str. Codegen will translate to
diff --git a/src/cxx.cc b/src/cxx.cc
index c9e215169..ac10df181 100644
--- a/src/cxx.cc
+++ b/src/cxx.cc
@@ -1,4 +1,5 @@
 #include "../include/cxx.h"
+#include <algorithm>
 #include <cassert>
 #include <cstring>
 #include <exception>
@@ -140,6 +141,30 @@ String::const_iterator String::cend() const noexcept {
   return this->data() + this->size();
 }
 
+bool String::operator==(const String &rhs) const noexcept {
+  return rust::Str(*this) == rust::Str(rhs);
+}
+
+bool String::operator!=(const String &rhs) const noexcept {
+  return rust::Str(*this) != rust::Str(rhs);
+}
+
+bool String::operator<(const String &rhs) const noexcept {
+  return rust::Str(*this) < rust::Str(rhs);
+}
+
+bool String::operator<=(const String &rhs) const noexcept {
+  return rust::Str(*this) <= rust::Str(rhs);
+}
+
+bool String::operator>(const String &rhs) const noexcept {
+  return rust::Str(*this) > rust::Str(rhs);
+}
+
+bool String::operator>=(const String &rhs) const noexcept {
+  return rust::Str(*this) >= rust::Str(rhs);
+}
+
 String::String(unsafe_bitcopy_t, const String &bits) noexcept
     : repr(bits.repr) {}
 
@@ -186,6 +211,39 @@ Str::const_iterator Str::cbegin() const noexcept { return this->ptr; }
 
 Str::const_iterator Str::cend() const noexcept { return this->ptr + this->len; }
 
+bool Str::operator==(const Str &rhs) const noexcept {
+  return this->len == rhs.len &&
+         std::equal(this->begin(), this->end(), rhs.begin());
+}
+
+bool Str::operator!=(const Str &rhs) const noexcept { return !(*this == rhs); }
+
+bool Str::operator<(const Str &rhs) const noexcept {
+  return std::lexicographical_compare(this->begin(), this->end(), rhs.begin(),
+                                      rhs.end());
+}
+
+bool Str::operator<=(const Str &rhs) const noexcept {
+  // std::mismatch(this->begin(), this->end(), rhs.begin(), rhs.end()), except
+  // without Undefined Behavior on C++11 if rhs is shorter than *this.
+  const_iterator liter = this->begin(), lend = this->end(), riter = rhs.begin(),
+                 rend = rhs.end();
+  while (liter != lend && riter != rend && *liter == *riter) {
+    ++liter, ++riter;
+  }
+  if (liter == lend) {
+    return true; // equal or *this is a prefix of rhs
+  } else if (riter == rend) {
+    return false; // rhs is a prefix of *this
+  } else {
+    return *liter <= *riter;
+  }
+}
+
+bool Str::operator>(const Str &rhs) const noexcept { return rhs < *this; }
+
+bool Str::operator>=(const Str &rhs) const noexcept { return rhs <= *this; }
+
 std::ostream &operator<<(std::ostream &os, const Str &s) {
   os.write(s.data(), s.size());
   return os;


Test patch:
diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc
index 340dc9c16..244c6c011 100644
--- a/tests/ffi/tests.cc
+++ b/tests/ffi/tests.cc
@@ -6,6 +6,7 @@
 #include <numeric>
 #include <stdexcept>
 #include <string>
+#include <tuple>
 
 extern "C" void cxx_test_suite_set_correct() noexcept;
 extern "C" tests::R *cxx_test_suite_get_box() noexcept;
@@ -656,6 +657,25 @@ extern "C" const char *cxx_run_test() noexcept {
   ASSERT(Shared{1} == Shared{1});
   ASSERT(Shared{1} != Shared{2});
 
+  rust::String first = "first", second = "second", sec = "sec";
+  bool (rust::String::*cmp)(const rust::String &) const;
+  bool first_first, first_second, sec_second, second_sec;
+  for (auto test : {
+    std::tuple<decltype(cmp), bool, bool, bool, bool>
+    {&rust::String::operator==, true, false, false, false},
+    {&rust::String::operator!=, false, true, true, true},
+    {&rust::String::operator<, false, true, true, false},
+    {&rust::String::operator<=, true, true, true, false},
+    {&rust::String::operator>, false, false, false, true},
+    {&rust::String::operator>=, true, false, false, true},
+  }) {
+    std::tie(cmp, first_first, first_second, sec_second, second_sec) = test;
+    ASSERT((first.*cmp)(first) == first_first);
+    ASSERT((first.*cmp)(second) == first_second);
+    ASSERT((sec.*cmp)(second) == sec_second);
+    ASSERT((second.*cmp)(sec) == second_sec);
+  }
+
   cxx_test_suite_set_correct();
   return nullptr;
 }