BLI: improve StringRef for C++17

Since C++17 there is also std::string_view, which is similar to StringRef.
This commit does a couple of things:
* New implicit conversions between StringRef and std::string_view.
* Support std::string_view in blender::DefaultHash.
* Support most of the methods that std::string_view has.
* Add StringRef::not_found which can be used instead of -1 in some places.
* Improve/fix the description at the top of BLI_string_ref.hh.

4 files changed, 197 insertions, 12 deletions

diff --git a/source/blender/blenlib/BLI_hash.hh b/source/blender/blenlib/BLI_hash.hh
index 8301dbb8e3a..2e3212cc83b 100644
--- a/source/blender/blenlib/BLI_hash.hh
+++ b/source/blender/blenlib/BLI_hash.hh
@@ -180,6 +180,13 @@ template<> struct DefaultHash<StringRefNull> {
   }
 };
 
+template<> struct DefaultHash<std::string_view> {
+  uint64_t operator()(StringRef value) const
+  {
+    return hash_string(value);
+  }
+};
+
 /**
  * While we cannot guarantee that the lower 4 bits of a pointer are zero, it is often the case.
  */
diff --git a/source/blender/blenlib/BLI_string_ref.hh b/source/blender/blenlib/BLI_string_ref.hh
index a217dc4c772..72803ebecda 100644
--- a/source/blender/blenlib/BLI_string_ref.hh
+++ b/source/blender/blenlib/BLI_string_ref.hh
@@ -37,13 +37,16 @@
  *   Both types are certainly very similar. The main benefit of using StringRef in Blender is that
  *   this allows us to add convenience methods at any time. Especially, when doing a lot of string
  *   manipulation, this helps to keep the code clean. Furthermore, we need StringRefNull anyway,
- *   because there is a lot of C code that expects null-terminated strings. Once we use C++17,
- *   implicit conversions to and from string_view can be added.
+ *   because there is a lot of C code that expects null-terminated strings. Conversion between
+ *   StringRef and string_view is very cheap and can be done at api boundaries at essentially no
+ *   cost. Another benefit of using StringRef is that it uses signed integers, thus developers
+ *   have to deal less with issues resulting from unsigned integers.
  */
 
 #include <cstring>
 #include <sstream>
 #include <string>
+#include <string_view>
 
 #include "BLI_span.hh"
 #include "BLI_utildefines.h"
@@ -66,6 +69,9 @@ class StringRefBase {
   }
 
  public:
+  /* Similar to string_view::npos, but signed. */
+  static constexpr int64_t not_found = -1;
+
   /**
    * Return the (byte-)length of the referenced string, without any null-terminator.
    */
@@ -74,6 +80,11 @@ class StringRefBase {
     return size_;
   }
 
+  bool is_empty() const
+  {
+    return size_ == 0;
+  }
+
   /**
    * Return a pointer to the start of the string.
    */
@@ -93,7 +104,12 @@ class StringRefBase {
    */
   operator std::string() const
   {
-    return std::string(data_, (size_t)size_);
+    return std::string(data_, static_cast<size_t>(size_));
+  }
+
+  operator std::string_view() const
+  {
+    return std::string_view(data_, static_cast<size_t>(size_));
   }
 
   const char *begin() const
@@ -113,7 +129,7 @@ class StringRefBase {
    */
   void unsafe_copy(char *dst) const
   {
-    memcpy(dst, data_, (size_t)size_);
+    memcpy(dst, data_, static_cast<size_t>(size_));
     dst[size_] = '\0';
   }
 
@@ -152,6 +168,41 @@ class StringRefBase {
   bool endswith(StringRef suffix) const;
 
   StringRef substr(int64_t start, const int64_t size) const;
+
+  /**
+   * Get the first char in the string. This invokes undefined behavior when the string is empty.
+   */
+  const char &front() const
+  {
+    BLI_assert(size_ >= 1);
+    return data_[0];
+  }
+
+  /**
+   * Get the last char in the string. This invokes undefined behavior when the string is empty.
+   */
+  const char &back() const
+  {
+    BLI_assert(size_ >= 1);
+    return data_[size_ - 1];
+  }
+
+  /**
+   * The behavior of those functions matches the standard library implementation of
+   * std::string_view.
+   */
+  int64_t find(char c, int64_t pos = 0) const;
+  int64_t find(StringRef str, int64_t pos = 0) const;
+  int64_t rfind(char c, int64_t pos = INT64_MAX) const;
+  int64_t rfind(StringRef str, int64_t pos = INT64_MAX) const;
+  int64_t find_first_of(StringRef chars, int64_t pos = 0) const;
+  int64_t find_first_of(char c, int64_t pos = 0) const;
+  int64_t find_last_of(StringRef chars, int64_t pos = INT64_MAX) const;
+  int64_t find_last_of(char c, int64_t pos = INT64_MAX) const;
+  int64_t find_first_not_of(StringRef chars, int64_t pos = 0) const;
+  int64_t find_first_not_of(char c, int64_t pos = 0) const;
+  int64_t find_last_not_of(StringRef chars, int64_t pos = INT64_MAX) const;
+  int64_t find_last_not_of(char c, int64_t pos = INT64_MAX) const;
 };
 
 /**
@@ -165,7 +216,8 @@ class StringRefNull : public StringRefBase {
   }
 
   /**
-   * Construct a StringRefNull from a null terminated c-string. The pointer must not point to NULL.
+   * Construct a StringRefNull from a null terminated c-string. The pointer must not point to
+   * NULL.
    */
   StringRefNull(const char *str) : StringRefBase(str, static_cast<int64_t>(strlen(str)))
   {
@@ -257,8 +309,14 @@ class StringRef : public StringRefBase {
   {
   }
 
+  StringRef(std::string_view view) : StringRefBase(view.data(), static_cast<int64_t>(view.size()))
+  {
+  }
+
   /**
-   * Return a new StringRef that does not contain the first n chars.
+   * Returns a new StringRef that does not contain the first n chars.
+   *
+   * This is similar to std::string_view::remove_prefix.
    */
   StringRef drop_prefix(const int64_t n) const
   {
@@ -268,8 +326,8 @@ class StringRef : public StringRefBase {
   }
 
   /**
-   * Return a new StringRef that with the given prefix being skipped.
-   * Asserts that the string begins with the given prefix.
+   * Return a new StringRef with the given prefix being skipped. This invokes undefined behavior if
+   * the string does not begin with the given prefix.
    */
   StringRef drop_prefix(StringRef prefix) const
   {
@@ -278,6 +336,18 @@ class StringRef : public StringRefBase {
   }
 
   /**
+   * Return a new StringRef that does not contain the last n chars.
+   *
+   * This is similar to std::string_view::remove_suffix.
+   */
+  StringRef drop_suffix(const int64_t n) const
+  {
+    BLI_assert(n >= 0);
+    BLI_assert(n <= size_);
+    return StringRef(data_, size_ - n);
+  }
+
+  /**
    * Get the char at the given index.
    */
   char operator[](int64_t index) const
@@ -312,6 +382,10 @@ inline std::string operator+(StringRef a, StringRef b)
   return std::string(a) + std::string(b);
 }
 
+/* This does not compare StringRef and std::string_view, because of ambiguous overloads. This is
+ * not a problem when std::string_view is only used at api boundaries. To compare a StringRef and a
+ * std::string_view, one should convert the std::string_view to StringRef (which is very cheap).
+ * Ideally, we only use StringRef in our code to avoid this problem alltogether. */
 inline bool operator==(StringRef a, StringRef b)
 {
   if (a.size() != b.size()) {
@@ -361,12 +435,82 @@ inline bool StringRefBase::endswith(StringRef suffix) const
 /**
  * Return a new #StringRef containing only a sub-string of the original string.
  */
-inline StringRef StringRefBase::substr(const int64_t start, const int64_t size) const
+inline StringRef StringRefBase::substr(const int64_t start,
+                                       const int64_t max_size = INT64_MAX) const
 {
-  BLI_assert(size >= 0);
+  BLI_assert(max_size >= 0);
   BLI_assert(start >= 0);
-  BLI_assert(start + size <= size_);
-  return StringRef(data_ + start, size);
+  const int64_t substr_size = std::min(max_size, size_ - start);
+  return StringRef(data_ + start, substr_size);
+}
+
+inline int64_t index_or_npos_to_int64(size_t index)
+{
+  /* The compiler will probably optimize this check away. */
+  if (index == std::string_view::npos) {
+    return StringRef::not_found;
+  }
+  return static_cast<int64_t>(index);
+}
+
+inline int64_t StringRefBase::find(char c, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(std::string_view(*this).find(c, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find(StringRef str, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(std::string_view(*this).find(str, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find_first_of(StringRef chars, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(
+      std::string_view(*this).find_first_of(chars, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find_first_of(char c, int64_t pos) const
+{
+  return this->find_first_of(StringRef(&c, 1), pos);
+}
+
+inline int64_t StringRefBase::find_last_of(StringRef chars, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(
+      std::string_view(*this).find_last_of(chars, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find_last_of(char c, int64_t pos) const
+{
+  return this->find_last_of(StringRef(&c, 1), pos);
+}
+
+inline int64_t StringRefBase::find_first_not_of(StringRef chars, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(
+      std::string_view(*this).find_first_not_of(chars, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find_first_not_of(char c, int64_t pos) const
+{
+  return this->find_first_not_of(StringRef(&c, 1), pos);
+}
+
+inline int64_t StringRefBase::find_last_not_of(StringRef chars, int64_t pos) const
+{
+  BLI_assert(pos >= 0);
+  return index_or_npos_to_int64(
+      std::string_view(*this).find_last_not_of(chars, static_cast<size_t>(pos)));
+}
+
+inline int64_t StringRefBase::find_last_not_of(char c, int64_t pos) const
+{
+  return this->find_last_not_of(StringRef(&c, 1), pos);
 }
 
 }  // namespace blender
diff --git a/source/blender/blenlib/tests/BLI_set_test.cc b/source/blender/blenlib/tests/BLI_set_test.cc
index 7bd0b258df8..df3f7ab544c 100644
--- a/source/blender/blenlib/tests/BLI_set_test.cc
+++ b/source/blender/blenlib/tests/BLI_set_test.cc
@@ -452,6 +452,16 @@ TEST(set, LookupKeyPtr)
   EXPECT_EQ(set.lookup_key_ptr({3, 50}), nullptr);
 }
 
+TEST(set, StringViewKeys)
+{
+  Set<std::string_view> set;
+  set.add("hello");
+  set.add("world");
+  EXPECT_FALSE(set.contains("worlds"));
+  EXPECT_TRUE(set.contains("world"));
+  EXPECT_TRUE(set.contains("hello"));
+}
+
 /**
  * Set this to 1 to activate the benchmark. It is disabled by default, because it prints a lot.
  */
diff --git a/source/blender/blenlib/tests/BLI_string_ref_test.cc b/source/blender/blenlib/tests/BLI_string_ref_test.cc
index d08c8a77455..2d488feff71 100644
--- a/source/blender/blenlib/tests/BLI_string_ref_test.cc
+++ b/source/blender/blenlib/tests/BLI_string_ref_test.cc
@@ -254,6 +254,14 @@ TEST(string_ref, DropPrefix)
   EXPECT_EQ(ref2, "t");
 }
 
+TEST(string_ref, DropSuffix)
+{
+  StringRef ref("test");
+  StringRef ref2 = ref.drop_suffix(1);
+  EXPECT_EQ(ref2.size(), 3);
+  EXPECT_EQ(ref2, "tes");
+}
+
 TEST(string_ref, Substr)
 {
   StringRef ref("hello world");
@@ -261,6 +269,8 @@ TEST(string_ref, Substr)
   EXPECT_EQ(ref.substr(4, 0), "");
   EXPECT_EQ(ref.substr(3, 4), "lo w");
   EXPECT_EQ(ref.substr(6, 5), "world");
+  EXPECT_EQ(ref.substr(8), "rld");
+  EXPECT_EQ(ref.substr(8, 100), "rld");
 }
 
 TEST(string_ref, Copy)
@@ -274,4 +284,18 @@ TEST(string_ref, Copy)
   EXPECT_EQ(ref, dst);
 }
 
+TEST(string_ref, FromStringView)
+{
+  std::string_view view = "hello";
+  StringRef ref = view;
+  EXPECT_EQ(ref, "hello");
+}
+
+TEST(string_ref, ToStringView)
+{
+  StringRef ref = "hello";
+  std::string_view view = ref;
+  EXPECT_EQ(view, "hello");
+}
+
 }  // namespace blender::tests