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

2016-10-19 Thread Marshall Clow via cfe-commits 
mclow.lists closed this revision.
mclow.lists marked 3 inline comments as done.
mclow.lists added a comment.

Landed as revision 284602


https://reviews.llvm.org/D25534



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2016-10-13 Thread 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  constexpr reverse_iterator(const reverse_iterator& 
u);
+template  constexpr reverse_iterator& operator (const 
reverse_iterator& 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 it1 = 
std::make_reverse_iterator(p);
+   constexpr std::reverse_iteratorit2 = 
(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`. 
Maybe:

```
using BaseIt = std::reverse_iterator;
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_iteratorit2 = 
(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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2016-10-12 Thread Marshall Clow via cfe-commits 
mclow.lists created this revision.
mclow.lists added reviewers: EricWF, lefticus, AntonBikineev.
mclow.lists added a subscriber: cfe-commits.

This just does the `reverse_iterator` bits of http://wg21.link/P0031 - not any 
of the other parts.
This duplicates some (but not all) of the work that was done in 
https://reviews.llvm.org/D20915 and https://reviews.llvm.org/D22584.

The win here (I think) is the extensive tests.


https://reviews.llvm.org/D25534

Files:
  include/iterator
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.cons/default.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.cons/iter.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.cons/reverse_iterator.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.make/make_reverse_iterator.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op!=/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op++/post.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op++/pre.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op+/difference_type.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op+=/difference_type.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op--/post.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op--/pre.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op-/difference_type.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op-=/difference_type.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op.star/op_star.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op=/reverse_iterator.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op==/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opdiff/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opgt/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opgt=/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.oplt/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.oplt=/test.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp
  
test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opsum/difference_type.pass.cpp

Index: test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opsum/difference_type.pass.cpp
===
--- test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opsum/difference_type.pass.cpp
+++ test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opsum/difference_type.pass.cpp
@@ -12,12 +12,15 @@
 // reverse_iterator
 
 // template 
-//   reverse_iterator
+//   constexpr reverse_iterator
 //   operator+(Iter::difference_type n, const reverse_iterator& x);
+//
+// constexpr in C++17
 
 #include 
 #include 
 
+#include "test_macros.h"
 #include "test_iterators.h"
 
 template 
@@ -34,4 +37,17 @@
 const char* s = "1234567890";
 test(random_access_iterator(s+5), 5, random_access_iterator(s));
 test(s+5, 5, s);
+
+#if TEST_STD_VER > 14
+	{
+		constexpr const char *p = "123456789";
+		typedef std::reverse_iterator RI;
+		constexpr RI it1 = std::make_reverse_iterator(p);
+		constexpr RI it2 = std::make_reverse_iterator(p + 5);
+		constexpr RI it3 = 5 + it2;
+		static_assert(it1 != it2, "");
+		static_assert(it1 == it3, "");
+		static_assert(it2 != it3, "");
+	}
+#endif
 }
Index: test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp
===
--- test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp
+++ test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp
@@ -11,7 +11,9 @@
 
 // reverse_iterator
 
-// pointer operator->() const;
+// constexpr pointer operator->() const;
+//
+// constexpr in C++17
 
 // Be sure to respect LWG 198:
 //http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#198
@@ -23,6 +25,8 @@
 #include 
 #include 
 
+#include