[libcxx-commits] [PATCH] D117400: [libc++] [test] Refactor iterators.common/ctor.pass.cpp

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp:34
+  static_assert( std::is_default_constructible_v<std::common_iterator<int*, sentinel_wrapper<int*>>>);
+  static_assert(!std::is_default_constructible_v<std::common_iterator<cpp17_input_iterator<int*>, sentinel_wrapper<cpp17_input_iterator<int*>>>>);
 
----------------
ldionne wrote:
> See, now you understand why I fixed `cpp17_input_iterator` to *not* be default constructible! 😄
I'll admit, it was convenient. ;)
I think what I wanted here was one default-constructible input iterator and one non-; which could have been `cpp17_input_iterator` and `cpp20_input_iterator`; except that `cpp20_input_iterator` is //also// not copyable, and I forget but I think that might disqualify it from use with `common_iterator`.


================
Comment at: libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp:75-76
+    using DerivedIt = std::common_iterator<Derived*, const Derived*>;
+    static_assert(std::is_convertible_v<DerivedIt, BaseIt>); // Derived* to Base*
+    static_assert(!std::is_constructible_v<DerivedIt, BaseIt>); // Base* to Derived*
+    Derived a[10];
----------------
ldionne wrote:
> `is_convertible` and `is_constructible` are different in that `is_convertible` won't work if the conversion/construction is explicit. I think this would be better:
> 
> ```
> static_assert( std::is_convertible_v<DerivedIt, BaseIt>); // Derived* to Base*
> static_assert(!std::is_convertible_v<BaseIt, DerivedIt>); // Base* to Derived*
> ```
> 
> Unless that isn't what you want (in which case I'd like to understand)?
I was being maybe too clever omitting "redundant" tests here: I wanted to test that `DerivedIt` was //implicitly// convertible to `BaseIt`, but `BaseIt` wasn't even //explicitly// convertible to `DerivedIt`. I could have (and now, will) test all four permutations:
```
static_assert( std::is_convertible_v<DerivedIt, BaseIt>); // Derived* to Base*
static_assert( std::is_constructible_v<BaseIt, DerivedIt>); // Derived* to Base*
static_assert(!std::is_convertible_v<BaseIt, DerivedIt>); // Base* to Derived*
static_assert(!std::is_constructible_v<DerivedIt, BaseIt>); // Base* to Derived*
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117400/new/

https://reviews.llvm.org/D117400