[ADT] Made numerous methods of ImmutableList const

[Szelethus]

Summary

Also added ImmutableList<T>::iterator::operator->.

[vsk]

unittests/ADT/ImmutableListTest.cpp

83

It might be simpler to use a shorter name here (maybe 'Unmodifiable'?) and to have a comment explaining that the struct has no move/copy constructors/assignment operators.

89

Why do the *-assign operators not return references?

103

Nit, please clang-format diffs.

[Szelethus]

Fixes according to inline comments.

[vsk]

Thanks, LGTM as well.

include/llvm/ADT/ImmutableList.h

public:
   public:
     iterator() = default;
     iterator(ImmutableList l) : L(l.getInternalPointer()) {}
 
     iterator& operator++() { L = L->getTail(); return *this; }
     bool operator==(const iterator& I) const { return L == I.L; }
     bool operator!=(const iterator& I) const { return L != I.L; }
     const value_type& operator*() const { return L->getHead(); }
+    const typename std::remove_reference<value_type>::type* operator->() const {
+      return &L->getHead();
+    }
 
     ImmutableList getList() const { return L; }
   };
 
   /// begin - Returns an iterator referring to the head of the list, or
   ///  an iterator denoting the end of the list if the list is empty.
   iterator begin() const { return iterator(X); }
 
public:
   ///  because it the contents of the list do not need to be compared.  Note
   ///  that you should only compare two lists created from the same
   ///  ImmutableListFactory.
   bool isEqual(const ImmutableList& L) const { return X == L.X; }
 
   bool operator==(const ImmutableList& L) const { return isEqual(L); }
 
   /// getHead - Returns the head of the list.
-  const T& getHead() {
+  const T& getHead() const {
     assert(!isEmpty() && "Cannot get the head of an empty list.");
     return X->getHead();
   }
 
   /// getTail - Returns the tail of the list, which is another (possibly empty)
   ///  ImmutableList.
-  ImmutableList getTail() {
+  ImmutableList getTail() const {
     return X ? X->getTail() : nullptr;
   }
 
   void Profile(FoldingSetNodeID& ID) const {
     ID.AddPointer(X);
   }
 };
 

unittests/ADT/ImmutableListTest.cpp

TEST_F(ImmutableListTest, OneElemIntListTest) {
   EXPECT_FALSE(L.contains(2));
   EXPECT_TRUE(L3.contains(2));
   EXPECT_EQ(2, L3.getHead());
 
   EXPECT_FALSE(L2 == L3);
   EXPECT_FALSE(L2.contains(2));
 }
 
+// We'll store references to objects of this type.
+struct Unmodifiable {
+  Unmodifiable() = default;
+
+  // We'll delete all of these special member functions to make sure no copy or
+  // move happens during insertation.
+  Unmodifiable(const Unmodifiable &) = delete;
+  Unmodifiable(const Unmodifiable &&) = delete;
+  Unmodifiable &operator=(const Unmodifiable &) = delete;
+  Unmodifiable &operator=(const Unmodifiable &&) = delete;
+
+  void doNothing() const {}
+
+  void Profile(FoldingSetNodeID &ID) const { ID.AddPointer(this); }
+};
+
+// Mostly just a check whether ImmutableList::iterator can be instantiated
+// with a reference type as a template argument.
+TEST_F(ImmutableListTest, ReferenceStoringTest) {
+  ImmutableList<const Unmodifiable &>::Factory f;
+
+  Unmodifiable N;
+  ImmutableList<const Unmodifiable &> L = f.create(N);
+  for (ImmutableList<const Unmodifiable &>::iterator It = L.begin(),
+                                                     E = L.end();
+       It != E; ++It) {
+    It->doNothing();
+  }
+}
+
 TEST_F(ImmutableListTest, CreatingIntListTest) {
   ImmutableList<Wrapper<int>>::Factory f;
 
   ImmutableList<Wrapper<int>> L = f.getEmptyList();
   ImmutableList<Wrapper<int>> L2 = f.create(3);
 
   EXPECT_FALSE(L2.isEmpty());
   EXPECT_TRUE(L2.getTail().isEmpty());