ADT: Move ArrayRef comparison operators into the class

[labath]

Summary

I don't fully understand all language nuances here, but this allows the
implicit ArrayRef conversions to kick in when e.g. comparing ArrayRef to
a SmallVector. That seems like a good thing.

[dblaikie]

Add the inverse equality test too, perhaps - EXPECT_EQ(V1, AR1) ? Though I realize that looks uninteresting & it is, apparently - that's the tricky case for when op overloads are members (though this change doesn't make it a member)

@rsmith - Could you explain why this change makes a difference? I understand why the original implementation (as a standalone function template) doesn't compile (because T can't be deduced for the ArrayRef<T> pattern when given a SmallVector<T>) but how does making it a friend help matters? & is this the right solution, or does it have any drawbacks/missing cases that should be considered? Should this technique be applied in other places?

[labath]

I've had to revert this because it causes compilation errors on MSVC. I expected this could be a problem, though the error itself is rather puzzling:

C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\xutility(2919): error C2893: Failed to specialize function template 'unknown-type std::equal_to<void>::operator ()(_Ty1 &&,_Ty2 &&) const'
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\xutility(2919): note: With the following template arguments:
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\xutility(2919): note: '_Ty1=const llvm::MDOperand &'
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\xutility(2919): note: '_Ty2=const llvm::MDOperand &'

So it MSVC is picking up this friend declaration (or at least, getting confused by it) even when neither of the types are ArrayRefs. That seems like a bug to me, though we may not be able to do anything about it.

(While working on this, I've also realized that I can use the equals member function to avoid explicit ArrayRef cast when I need to compare a ArrayRef with a different type).

llvm/trunk/include/llvm/ADT/ArrayRef.h

public:
     /// @}
     /// @name Conversion operators
     /// @{
     operator std::vector<T>() const {
       return std::vector<T>(Data, Data+Length);
     }
 
     /// @}
+    /// @name Comparison operators
+    /// @{
+
+    friend bool operator==(ArrayRef LHS, ArrayRef RHS) {
+      return LHS.equals(RHS);
+    }
+
+    friend bool operator!=(ArrayRef LHS, ArrayRef RHS) { return !(LHS == RHS); }
+
+    /// @}
   };
 
   /// MutableArrayRef - Represent a mutable reference to an array (0 or more
   /// elements consecutively in memory), i.e. a start pointer and a length.  It
   /// allows various APIs to take and modify consecutive elements easily and
   /// conveniently.
   ///
   /// This class does not own the underlying data, it is expected to be used in
namespace llvm {
   }
 
   /// Construct a MutableArrayRef from a pointer and length.
   template<typename T>
   MutableArrayRef<T> makeMutableArrayRef(T *data, size_t length) {
     return MutableArrayRef<T>(data, length);
   }
 
-  /// @}
-  /// @name ArrayRef Comparison Operators
-  /// @{
-
-  template<typename T>
-  inline bool operator==(ArrayRef<T> LHS, ArrayRef<T> RHS) {
-    return LHS.equals(RHS);
-  }
-
-  template<typename T>
-  inline bool operator!=(ArrayRef<T> LHS, ArrayRef<T> RHS) {
-    return !(LHS == RHS);
-  }
-
-  /// @}
-
   // ArrayRefs can be treated like a POD type.
   template <typename T> struct isPodLike;
   template <typename T> struct isPodLike<ArrayRef<T>> {
     static const bool value = true;
   };
 
   template <typename T> hash_code hash_value(ArrayRef<T> S) {
     return hash_combine_range(S.begin(), S.end());
   }
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_ARRAYREF_H

llvm/trunk/unittests/ADT/ArrayRefTest.cpp

TEST(ArrayRefTest, Equals) {
   ArrayRef<int> AR1a = AR1.drop_back();
   EXPECT_TRUE(AR1a.equals({1, 2, 3, 4, 5, 6, 7}));
   EXPECT_FALSE(AR1a.equals({1, 2, 3, 4, 5, 6, 7, 8}));
 
   ArrayRef<int> AR1b = AR1a.slice(2, 4);
   EXPECT_TRUE(AR1b.equals({3, 4, 5, 6}));
   EXPECT_FALSE(AR1b.equals({2, 3, 4, 5, 6}));
   EXPECT_FALSE(AR1b.equals({3, 4, 5, 6, 7}));
+
+  SmallVector<int, 8> V1{1, 2, 3, 4, 5, 6, 7, 8};
+  EXPECT_EQ(AR1, V1);
+  EXPECT_EQ(V1, AR1);
 }
 
 TEST(ArrayRefTest, EmptyEquals) {
   EXPECT_TRUE(ArrayRef<unsigned>() == ArrayRef<unsigned>());
 }
 
 TEST(ArrayRefTest, ConstConvert) {
   int buf[4];