[glibmm] Use Glib::UStringView with Glib::ustring::compare()

[Kjell Ahlstedt <kjellahl src gnome org>]

commit 729b1ae95d5d05b78741eeef1d635f95b19d790b
Author: Thomas Holder <thomas holder schrodinger com>
Date:   Sat Nov 30 12:10:22 2019 +0100

    Use Glib::UStringView with Glib::ustring::compare()
    
    and with the comparison operators, operator==(), etc.
    Add the glibmm_ustring_compare test case.
    
    https://gitlab.gnome.org/GNOME/glibmm/issues/64
    https://gitlab.gnome.org/GNOME/glibmm/issues/65

 glib/glibmm/ustring.cc               |  21 +--
 glib/glibmm/ustring.h                | 249 ++++++++++++++++-------------------
 tests/Makefile.am                    |   3 +-
 tests/glibmm_ustring_compare/main.cc | 115 ++++++++++++++++
 4 files changed, 232 insertions(+), 156 deletions(-)
---
diff --git a/glib/glibmm/ustring.cc b/glib/glibmm/ustring.cc
index d17107a2..1a5026f4 100644
--- a/glib/glibmm/ustring.cc
+++ b/glib/glibmm/ustring.cc
@@ -33,6 +33,7 @@ namespace
 {
 
 using Glib::ustring;
+using Glib::UStringView;
 
 // Little helper to make the conversion from gunichar to UTF-8 a one-liner.
 //
@@ -737,21 +738,15 @@ ustring::erase(ustring::iterator pbegin, ustring::iterator pend)
 /**** Glib::ustring::compare() *********************************************/
 
 int
-ustring::compare(const ustring& rhs) const
+ustring::compare(UStringView rhs) const
 {
-  return g_utf8_collate(string_.c_str(), rhs.string_.c_str());
+  return g_utf8_collate(string_.c_str(), rhs.c_str());
 }
 
 int
-ustring::compare(const char* rhs) const
+ustring::compare(ustring::size_type i, ustring::size_type n, UStringView rhs) const
 {
-  return g_utf8_collate(string_.c_str(), rhs);
-}
-
-int
-ustring::compare(ustring::size_type i, ustring::size_type n, const ustring& rhs) const
-{
-  return ustring(*this, i, n).compare(rhs);
+  return ustring(*this, i, n).compare(rhs.c_str());
 }
 
 int
@@ -768,12 +763,6 @@ ustring::compare(
   return ustring(*this, i, n).compare(ustring(rhs, n2));
 }
 
-int
-ustring::compare(ustring::size_type i, ustring::size_type n, const char* rhs) const
-{
-  return ustring(*this, i, n).compare(rhs);
-}
-
 /**** Glib::ustring -- index access ****************************************/
 
 ustring::value_type ustring::operator[](ustring::size_type i) const
diff --git a/glib/glibmm/ustring.h b/glib/glibmm/ustring.h
index 2ce10c77..38414c9e 100644
--- a/glib/glibmm/ustring.h
+++ b/glib/glibmm/ustring.h
@@ -27,10 +27,99 @@
 #include <iterator>
 #include <sstream>
 #include <string>
+#include <type_traits>
 
 namespace Glib
 {
 
+class ustring;
+
+//********** Glib::StdStringView and Glib::UStringView *************
+
+// It would be possible to replace StdStringView and UStringView with a
+// template class BasicStringView + two type aliases defining StdStringView
+// and UStringView. But Doxygen don't generate links to type aliases.
+//
+// It would also be possible to replace StdStringView and UStringView with
+// a StringView class with 3 constructors, taking const std::string&,
+// const Glib::ustring& and const char*, respectively. The split into two classes
+// is by design. Using the wrong string class shall not be as easy as using
+// the right string class.
+
+/** Helper class to avoid unnecessary string copying in function calls.
+ *
+ * A %Glib::StdStringView holds a const char pointer. It can be used as an argument
+ * type in a function that passes a const char pointer to a C function.
+ *
+ * Unlike std::string_view, %Glib::StdStringView shall be used only for
+ * null-terminated strings.
+ * @code
+ * std::string f1(Glib::StdStringView s1, Glib::StdStringView s2);
+ * // can be used instead of
+ * std::string f2(const std::string& s1, const std::string& s2);
+ * @endcode
+ * The strings are not copied when f1() is called with string literals.
+ * @code
+ * auto r1 = f1("string 1", "string 2");
+ * @endcode
+ * To pass a Glib::ustring to a function taking a %Glib::StdStringView, you may have
+ * to use Glib::ustring::c_str().
+ * @code
+ * std::string str = "non-UTF8 string";
+ * Glib::ustring ustr = "UTF8 string";
+ * auto r1 = f1(str, ustr.c_str());
+ * @endcode
+ *
+ * @newin{2,64}
+ */
+class StdStringView
+{
+public:
+  StdStringView(const std::string& s) : pstring_(s.c_str()) {}
+  StdStringView(const char* s) : pstring_(s) {}
+  const char* c_str() const { return pstring_; }
+private:
+  const char* pstring_;
+};
+
+/** Helper class to avoid unnecessary string copying in function calls.
+ *
+ * A %Glib::UStringView holds a const char pointer. It can be used as an argument
+ * type in a function that passes a const char pointer to a C function.
+ *
+ * Unlike std::string_view, %Glib::UStringView shall be used only for
+ * null-terminated strings.
+ * @code
+ * Glib::ustring f1(Glib::UStringView s1, Glib::UStringView s2);
+ * // can be used instead of
+ * Glib::ustring f2(const Glib::ustring& s1, const Glib::ustring& s2);
+ * @endcode
+ * The strings are not copied when f1() is called with string literals.
+ * @code
+ * auto r1 = f1("string 1", "string 2");
+ * @endcode
+ * To pass a std::string to a function taking a %Glib::UStringView, you may have
+ * to use std::string::c_str().
+ * @code
+ * std::string str = "non-UTF8 string";
+ * Glib::ustring ustr = "UTF8 string";
+ * auto r1 = f1(str.c_str(), ustr);
+ * @endcode
+ *
+ * @newin{2,64}
+ */
+class UStringView
+{
+public:
+  inline UStringView(const Glib::ustring& s);
+  UStringView(const char* s) : pstring_(s) {}
+  const char* c_str() const { return pstring_; }
+private:
+  const char* pstring_;
+};
+
+//***************************************************
+
 #ifndef DOXYGEN_SHOULD_SKIP_THIS
 #ifndef GLIBMM_HAVE_STD_ITERATOR_TRAITS
 
@@ -428,12 +517,10 @@ public:
   //! @name Compare and collate.
   //! @{
 
-  int compare(const ustring& rhs) const;
-  int compare(const char* rhs) const;
-  int compare(size_type i, size_type n, const ustring& rhs) const;
+  int compare(UStringView rhs) const;
+  int compare(size_type i, size_type n, UStringView rhs) const;
   int compare(size_type i, size_type n, const ustring& rhs, size_type i2, size_type n2) const;
   int compare(size_type i, size_type n, const char* rhs, size_type n2) const;
-  int compare(size_type i, size_type n, const char* rhs) const;
 
   /*! Create a unique sorting key for the UTF-8 string.  If you need to
    * compare UTF-8 strings regularly, e.g. for sorted containers such as
@@ -1355,127 +1442,91 @@ swap(ustring& lhs, ustring& rhs)
 /**** Glib::ustring -- comparison operators ********************************/
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator==(const ustring& lhs, const ustring& rhs)
+operator==(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) == 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator==(const ustring& lhs, const char* rhs)
-{
-  return (lhs.compare(rhs) == 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator==(const char* lhs, const ustring& rhs)
+operator==(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) == 0);
 }
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator!=(const ustring& lhs, const ustring& rhs)
-{
-  return (lhs.compare(rhs) != 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator!=(const ustring& lhs, const char* rhs)
+operator!=(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) != 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator!=(const char* lhs, const ustring& rhs)
+operator!=(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) != 0);
 }
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator<(const ustring& lhs, const ustring& rhs)
+operator<(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) < 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator<(const ustring& lhs, const char* rhs)
-{
-  return (lhs.compare(rhs) < 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator<(const char* lhs, const ustring& rhs)
+operator<(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) > 0);
 }
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator>(const ustring& lhs, const ustring& rhs)
+operator>(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) > 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator>(const ustring& lhs, const char* rhs)
-{
-  return (lhs.compare(rhs) > 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator>(const char* lhs, const ustring& rhs)
+operator>(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) < 0);
 }
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator<=(const ustring& lhs, const ustring& rhs)
+operator<=(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) <= 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator<=(const ustring& lhs, const char* rhs)
-{
-  return (lhs.compare(rhs) <= 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator<=(const char* lhs, const ustring& rhs)
+operator<=(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) >= 0);
 }
 
 /** @relates Glib::ustring */
+template <typename T, typename = std::enable_if_t<!std::is_base_of_v<ustring, T>>>
 inline bool
-operator>=(const ustring& lhs, const ustring& rhs)
-{
-  return (lhs.compare(rhs) >= 0);
-}
-
-/** @relates Glib::ustring */
-inline bool
-operator>=(const ustring& lhs, const char* rhs)
+operator>=(const ustring& lhs, const T& rhs)
 {
   return (lhs.compare(rhs) >= 0);
 }
 
 /** @relates Glib::ustring */
 inline bool
-operator>=(const char* lhs, const ustring& rhs)
+operator>=(UStringView lhs, const ustring& rhs)
 {
   return (rhs.compare(lhs) <= 0);
 }
@@ -1566,87 +1617,7 @@ operator+(char lhs, const ustring& rhs)
 
 //********** Glib::StdStringView and Glib::UStringView *************
 
-// It would be possible to replace StdStringView and UStringView with a
-// template class BasicStringView + two type aliases defining StdStringView
-// and UStringView. But Doxygen don't generate links to type aliases.
-//
-// It would also be possible to replace StdStringView and UStringView with
-// a StringView class with 3 constructors, taking const std::string&,
-// const Glib::ustring& and const char*, respectively. The split into two classes
-// is by design. Using the wrong string class shall not be as easy as using
-// the right string class.
-
-/** Helper class to avoid unnecessary string copying in function calls.
- *
- * A %Glib::StdStringView holds a const char pointer. It can be used as an argument
- * type in a function that passes a const char pointer to a C function.
- *
- * Unlike std::string_view, %Glib::StdStringView shall be used only for
- * null-terminated strings.
- * @code
- * std::string f1(Glib::StdStringView s1, Glib::StdStringView s2);
- * // can be used instead of
- * std::string f2(const std::string& s1, const std::string& s2);
- * @endcode
- * The strings are not copied when f1() is called with string literals.
- * @code
- * auto r1 = f1("string 1", "string 2");
- * @endcode
- * To pass a Glib::ustring to a function taking a %Glib::StdStringView, you may have
- * to use Glib::ustring::c_str().
- * @code
- * std::string str = "non-UTF8 string";
- * Glib::ustring ustr = "UTF8 string";
- * auto r1 = f1(str, ustr.c_str());
- * @endcode
- *
- * @newin{2,64}
- */
-class StdStringView
-{
-public:
-  StdStringView(const std::string& s) : pstring_(s.c_str()) {}
-  StdStringView(const char* s) : pstring_(s) {}
-  const char* c_str() const { return pstring_; }
-private:
-  const char* pstring_;
-};
-
-/** Helper class to avoid unnecessary string copying in function calls.
- *
- * A %Glib::UStringView holds a const char pointer. It can be used as an argument
- * type in a function that passes a const char pointer to a C function.
- *
- * Unlike std::string_view, %Glib::UStringView shall be used only for
- * null-terminated strings.
- * @code
- * Glib::ustring f1(Glib::UStringView s1, Glib::UStringView s2);
- * // can be used instead of
- * Glib::ustring f2(const Glib::ustring& s1, const Glib::ustring& s2);
- * @endcode
- * The strings are not copied when f1() is called with string literals.
- * @code
- * auto r1 = f1("string 1", "string 2");
- * @endcode
- * To pass a std::string to a function taking a %Glib::UStringView, you may have
- * to use std::string::c_str().
- * @code
- * std::string str = "non-UTF8 string";
- * Glib::ustring ustr = "UTF8 string";
- * auto r1 = f1(str.c_str(), ustr);
- * @endcode
- *
- * @newin{2,64}
- */
-class UStringView
-{
-public:
-  UStringView(const Glib::ustring& s) : pstring_(s.c_str()) {}
-  UStringView(const char* s) : pstring_(s) {}
-  const char* c_str() const { return pstring_; }
-private:
-  const char* pstring_;
-};
+inline UStringView::UStringView(const ustring& s) : pstring_(s.c_str()) {}
 
 } // namespace Glib
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 68effc10..2ad6e858 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -40,7 +40,7 @@ check_PROGRAMS =                              \
        glibmm_objectbase/test                  \
        glibmm_objectbase_move/test                     \
        glibmm_regex/test                       \
-       glibmm_ustring_compose/test             \
+       glibmm_ustring_compare/test             \
        glibmm_ustring_format/test              \
        glibmm_ustring_sprintf/test             \
        glibmm_value/test                       \
@@ -115,6 +115,7 @@ glibmm_objectbase_test_SOURCES           = glibmm_objectbase/main.cc \
 glibmm_objectbase_move_test_SOURCES      = glibmm_objectbase_move/main.cc \
                                           glibmm_objectbase/test_derived_objectbase.h \
                                           glibmm_object/test_derived_object.h
+glibmm_ustring_compare_test_SOURCES      = glibmm_ustring_compare/main.cc
 glibmm_ustring_compose_test_SOURCES      = glibmm_ustring_compose/main.cc
 glibmm_ustring_format_test_SOURCES       = glibmm_ustring_format/main.cc
 glibmm_ustring_sprintf_test_SOURCES      = glibmm_ustring_sprintf/main.cc
diff --git a/tests/glibmm_ustring_compare/main.cc b/tests/glibmm_ustring_compare/main.cc
new file mode 100644
index 00000000..349a2741
--- /dev/null
+++ b/tests/glibmm_ustring_compare/main.cc
@@ -0,0 +1,115 @@
+#include <glibmm.h>
+
+#include <iostream>
+
+// Helper class to check for non-existing overload
+template<typename T>
+struct Convertible
+{
+  Convertible(const T&){};
+};
+
+static bool expect_missing_overload = false;
+
+void
+operator==(Convertible<std::string> const&, Glib::ustring const&)
+{
+  g_assert_true(expect_missing_overload);
+  expect_missing_overload = false;
+}
+
+int
+main(int, char**)
+{
+  // allocating
+  static_assert(std::is_convertible<const char*, Glib::ustring>::value, "");
+  // non-allocating
+  static_assert(std::is_convertible<const char*, Glib::UStringView>::value, "");
+  static_assert(std::is_convertible<Glib::ustring, Glib::UStringView>::value, "");
+  // deliberately omitted
+  static_assert(!std::is_convertible<Glib::UStringView, Glib::ustring>::value, "");
+  static_assert(!std::is_convertible<Glib::UStringView, const char *>::value, "");
+
+  const char *cstr1 = "Hello";
+  const char *cstr2 = "World";
+  const char *cstr12 = "HelloWorld";
+  const char *cstr12_25 = "lloWo"; // cstr12[2:2 + 5]
+
+  Glib::ustring ustr1 = cstr1;
+  Glib::ustring ustr2 = cstr2;
+  Glib::ustring ustr12 = cstr12;
+  Glib::ustring ustr12_25 = cstr12_25;
+
+  Glib::UStringView vstr1 = cstr1;
+  Glib::UStringView vstr2 = cstr2;
+  Glib::UStringView vstr12_25 = cstr12_25;
+
+  g_assert_true(ustr1.compare(ustr1) == 0);
+  g_assert_true(ustr1.compare(cstr1) == 0);
+  g_assert_true(ustr1.compare(vstr1) == 0);
+
+  g_assert_true(ustr1.compare(ustr2) < 0);
+  g_assert_true(ustr1.compare(cstr2) < 0);
+  g_assert_true(ustr1.compare(vstr2) < 0);
+
+  g_assert_true(ustr12.compare(2, 5, ustr12_25) == 0);
+  g_assert_true(ustr12.compare(2, 5, cstr12_25) == 0);
+  g_assert_true(ustr12.compare(2, 5, vstr12_25) == 0);
+
+  g_assert_true(ustr1 == ustr1);
+  g_assert_true(ustr1 == cstr1);
+  g_assert_true(ustr1 == vstr1);
+  g_assert_true(cstr1 == ustr1);
+  g_assert_true(vstr1 == ustr1);
+
+  g_assert_true(ustr2 != ustr1);
+  g_assert_true(ustr2 != cstr1);
+  g_assert_true(ustr2 != vstr1);
+  g_assert_true(cstr2 != ustr1);
+  g_assert_true(vstr2 != ustr1);
+
+  g_assert_true(ustr2 > ustr1);
+  g_assert_true(ustr2 > cstr1);
+  g_assert_true(ustr2 > vstr1);
+  g_assert_true(cstr2 > ustr1);
+  g_assert_true(vstr2 > ustr1);
+
+  g_assert_false(ustr2 < ustr1);
+  g_assert_false(ustr2 < cstr1);
+  g_assert_false(ustr2 < vstr1);
+  g_assert_false(cstr2 < ustr1);
+  g_assert_false(vstr2 < ustr1);
+
+  g_assert_true(ustr2 >= ustr1);
+  g_assert_true(ustr2 >= cstr1);
+  g_assert_true(ustr2 >= vstr1);
+  g_assert_true(cstr2 >= ustr1);
+  g_assert_true(vstr2 >= ustr1);
+
+  g_assert_false(ustr2 <= ustr1);
+  g_assert_false(ustr2 <= cstr1);
+  g_assert_false(ustr2 <= vstr1);
+  g_assert_false(cstr2 <= ustr1);
+  g_assert_false(vstr2 <= ustr1);
+
+  // no conversions between std::string and UStringView and no comparison between
+  // std::string and ustring/UStringView
+
+  static_assert(!std::is_convertible<std::string, Glib::UStringView>::value, "");
+  static_assert(!std::is_convertible<Glib::UStringView, std::string>::value, "");
+
+  std::string sstr1 = cstr1;
+
+  // Would not compile without the helper overload
+  expect_missing_overload = true;
+  sstr1 == ustr1;
+  g_assert_false(expect_missing_overload);
+
+  // Doesn't compile because of missing Glib::ustring::compare overload (expected), but
+  // unfortunately not testable like the other way round.
+#if 0
+  ustr1 == sstr1;
+#endif
+
+  return EXIT_SUCCESS;
+}