From c120ead9dff00a39fc66f5655fb911447fb54eb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Tue, 20 Feb 2024 17:52:14 +0100
Subject: [PATCH] BLI: Clarify and test StringRefNull comparisons

Add some comments that clarify that `StringRefNull` can be compared with
other `StringRefNull` and with `StringRef` instances.

Add unit tests that cover these cases.

Pull Request: https://projects.blender.org/blender/blender/pulls/118515
---
 source/blender/blenlib/BLI_string_ref.hh      |  9 ++-
 .../blenlib/tests/BLI_string_ref_test.cc      | 65 +++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/source/blender/blenlib/BLI_string_ref.hh b/source/blender/blenlib/BLI_string_ref.hh
index 40146961b10..9f7c6402cda 100644
--- a/source/blender/blenlib/BLI_string_ref.hh
+++ b/source/blender/blenlib/BLI_string_ref.hh
@@ -105,6 +105,8 @@ class StringRefBase {
 
 /**
  * References a null-terminated const char array.
+ *
+ * StringRefNull can be compared with StringRef and StringRefNull.
  */
 class StringRefNull : public StringRefBase {
 
@@ -120,6 +122,8 @@ class StringRefNull : public StringRefBase {
 
 /**
  * References a const char array. It might not be null terminated.
+ *
+ * StringRef can be compared with StringRef and StringRefNull.
  */
 class StringRef : public StringRefBase {
  public:
@@ -588,7 +592,10 @@ inline std::string operator+(StringRef a, StringRef 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 altogether. */
+ * Ideally, we only use StringRef in our code to avoid this problem altogether.
+ *
+ * NOTE: these functions are also suitable for StringRefNull comparisons, as these are
+ * implicitly converted to StringRef by the compiler. */
 constexpr bool operator==(StringRef a, StringRef b)
 {
   return std::string_view(a) == std::string_view(b);
diff --git a/source/blender/blenlib/tests/BLI_string_ref_test.cc b/source/blender/blenlib/tests/BLI_string_ref_test.cc
index a59714de08c..7f2559286b9 100644
--- a/source/blender/blenlib/tests/BLI_string_ref_test.cc
+++ b/source/blender/blenlib/tests/BLI_string_ref_test.cc
@@ -34,6 +34,71 @@ TEST(string_ref_null, CStringLengthConstructor)
   EXPECT_EQ(ref.data(), str);
 }
 
+TEST(string_ref_null, CompareEqual)
+{
+  StringRefNull ref1("test");
+  StringRefNull ref2("test");
+  StringRefNull ref3("other");
+  EXPECT_TRUE(ref1 == ref2);
+  EXPECT_FALSE(ref1 == ref3);
+  EXPECT_TRUE(ref1 != ref3);
+  EXPECT_FALSE(ref1 != ref2);
+}
+
+TEST(string_ref_null, CompareEqualCharPtr1)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE(ref == "test");
+  EXPECT_FALSE(ref == "other");
+  EXPECT_TRUE(ref != "other");
+  EXPECT_FALSE(ref != "test");
+}
+
+TEST(string_ref_null, CompareEqualCharPtr2)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE("test" == ref);
+  EXPECT_FALSE("other" == ref);
+  EXPECT_TRUE(ref != "other");
+  EXPECT_FALSE(ref != "test");
+}
+
+TEST(string_ref_null, CompareEqualString1)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE(ref == std::string("test"));
+  EXPECT_FALSE(ref == std::string("other"));
+  EXPECT_TRUE(ref != std::string("other"));
+  EXPECT_FALSE(ref != std::string("test"));
+}
+
+TEST(string_ref_null, CompareEqualString2)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE(std::string("test") == ref);
+  EXPECT_FALSE(std::string("other") == ref);
+  EXPECT_TRUE(std::string("other") != ref);
+  EXPECT_FALSE(std::string("test") != ref);
+}
+
+TEST(string_ref_null, CompareEqualStringRef1)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE(ref == StringRef("test"));
+  EXPECT_FALSE(ref == StringRef("other"));
+  EXPECT_TRUE(ref != StringRef("other"));
+  EXPECT_FALSE(ref != StringRef("test"));
+}
+
+TEST(string_ref_null, CompareEqualStringRef2)
+{
+  StringRefNull ref("test");
+  EXPECT_TRUE(StringRef("test") == ref);
+  EXPECT_FALSE(StringRef("other") == ref);
+  EXPECT_TRUE(StringRef("other") != ref);
+  EXPECT_FALSE(StringRef("test") != ref);
+}
+
 TEST(string_ref, DefaultConstructor)
 {
   StringRef ref;