[PATCH] D25534: Implement part of P0031; adding `constexpr` to `reverse_iterator`

[Eric Fiselier via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM modulo inline comments.



================
Comment at: include/iterator:95
+    template <class U> constexpr reverse_iterator(const reverse_iterator<U>& u);
+    template <class U> constexpr reverse_iterator& operator (const reverse_iterator<U>& u);
+    constexpr Iterator base() const;
----------------
Missing an `=` here. 


================
Comment at: test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op=/reverse_iterator.pass.cpp:53
+		constexpr std::reverse_iterator<const Derived *>     it1 = std::make_reverse_iterator(p);
+		constexpr std::reverse_iterator<const Base *>        it2 = (std::make_reverse_iterator(p) = it1);
+        static_assert(it2.base() == p);
----------------
This actually tests the copy assignment operator since `std::make_reverse_iterator(p)` returns `reverse_iterator<const Derived *>`. 
Maybe:

```
using BaseIt = std::reverse_iterator<const Base *>;
BaseIt it2 = (BaseIt{nullptr} = it1); 
```

It would also be nice if `p` was non-null so we could test that the actual assignment took place.


================
Comment at: test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op=/reverse_iterator.pass.cpp:54
+		constexpr std::reverse_iterator<const Base *>        it2 = (std::make_reverse_iterator(p) = it1);
+        static_assert(it2.base() == p);
+	}
----------------
The static assert is missing a message and is poorly aligned.


================
Comment at: test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp:113
+
+		static_assert(it1->get() == gC.get(), "");
+	}
----------------
How about just:

```
 constexpr const char *p = "123456789";
 constexpr auto it = std::make_reverse_iterator(p+1);
 static_assert(it.operator->() == p, "");
```


https://reviews.llvm.org/D25534