[ADT] Implement ArrayRef::operator< and other comparisons

[jayfoad]

Order ArrayRefs using std::lexicographical_compare, just like
std::vector and SmallVector.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Jay Foad (jayfoad)

Changes

Order ArrayRefs using std::lexicographical_compare, just like
std::vector and SmallVector.

---

Full diff: https://github.com/llvm/llvm-project/pull/147277.diff

2 Files Affected:

• (modified) llvm/include/llvm/ADT/ArrayRef.h (+21)
• (modified) llvm/unittests/ADT/ArrayRefTest.cpp (+21)

diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 8091be2036642..ff8bdb8b4fec4 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -565,6 +565,27 @@ namespace llvm {
     return !(LHS == RHS);
   }
 
+  template <typename T>
+  inline bool operator<(ArrayRef<T> LHS, ArrayRef<T> RHS) {
+    return std::lexicographical_compare(LHS.begin(), LHS.end(), RHS.begin(),
+                                        RHS.end());
+  }
+
+  template <typename T>
+  inline bool operator>(ArrayRef<T> LHS, ArrayRef<T> RHS) {
+    return RHS < LHS;
+  }
+
+  template <typename T>
+  inline bool operator<=(ArrayRef<T> LHS, ArrayRef<T> RHS) {
+    return !(LHS > RHS);
+  }
+
+  template <typename T>
+  inline bool operator>=(ArrayRef<T> LHS, ArrayRef<T> RHS) {
+    return !(LHS < RHS);
+  }
+
   /// @}
 
   template <typename T> hash_code hash_value(ArrayRef<T> S) {
diff --git a/llvm/unittests/ADT/ArrayRefTest.cpp b/llvm/unittests/ADT/ArrayRefTest.cpp
index b5fc0a100571e..736c8fbb26b38 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -231,6 +231,27 @@ TEST(ArrayRefTest, EmptyEquals) {
   EXPECT_TRUE(ArrayRef<unsigned>() == ArrayRef<unsigned>());
 }
 
+TEST(ArrayRefTest, Compare) {
+  ArrayRef<char> Ban("Ban");
+  ArrayRef<char> Banana("Banana");
+  ArrayRef<char> Band("Band");
+
+  EXPECT_TRUE(Ban < Banana);
+  EXPECT_TRUE(Ban <= Banana);
+  EXPECT_FALSE(Ban > Banana);
+  EXPECT_FALSE(Ban >= Banana);
+
+  EXPECT_FALSE(Banana < Banana);
+  EXPECT_TRUE(Banana <= Banana);
+  EXPECT_FALSE(Banana > Banana);
+  EXPECT_TRUE(Banana >= Banana);
+
+  EXPECT_TRUE(Banana < Band);
+  EXPECT_TRUE(Banana <= Band);
+  EXPECT_FALSE(Banana > Band);
+  EXPECT_FALSE(Banana >= Band);
+}
+
 TEST(ArrayRefTest, ConstConvert) {
   int buf[4];
   for (int i = 0; i < 4; ++i)

[artagnon]

Looks good, but can you show a usecase?

[jayfoad]

Looks good, but can you show a usecase?

I'm working on a patch that uses it here, changing Members from a pointer-to-SmallVector to an ArrayRef. I guess this patch can wait until that one is ready for review.

[dwblaikie]

Does std::span have these operators, and if it does, are they SFINAE-safe there? And if so, should they be made SFINAE-safe here too?

[jayfoad]

Does std::span have these operators, and if it does, are they SFINAE-safe there? And if so, should they be made SFINAE-safe here too?

I think you might be mistaking me for a C++ expert :)

As far as I can tell from https://en.cppreference.com/w/cpp/container/span.html, std::span does not have these operators. But note that ArrayRef already had operator==, which std::span does not.

[dwblaikie]

Does std::span have these operators, and if it does, are they SFINAE-safe there? And if so, should they be made SFINAE-safe here too?

I think you might be mistaking me for a C++ expert :)

As far as I can tell from https://en.cppreference.com/w/cpp/container/span.html, std::span does not have these operators. But note that ArrayRef already had operator==, which std::span does not.

Hmm, yeah... bit of a pity that this is a direction that'll take us longer/make it harder to migrate to the standard thing...

( https://brevzin.github.io/c++/2020/03/30/span-comparisons/ discusses some of the complexities of implementing these things in their full generality, including the SFINAE stuff (see the bit about REQUIRES))

Eh, for now - guess we'll give it a go, cross the bridge about SFINAE when we need it, and/or migrate to non-members/something else when we want to migrate to std::span one day.

[kuhar]

and/or migrate to non-members/something else when we want to migrate to std::span one day.

I'm confused, this is implemented as free functions already.

[dwblaikie]

and/or migrate to non-members/something else when we want to migrate to std::span one day.

I'm confused, this is implemented as free functions already.

Sorry, as free functions that don't require the definitions to be in the matching namespace/use ADL - if we switched to std::span, we couldn't keep these operators, since they'd need to go in the std namespace to be effective, which we couldn't do. So we'd probably end up needing named functions, I guess? But I'm not 100% sure what we'd do, just pretty sure it couldn't be this direction.