[libcxx-commits] [PATCH] D117449: [libcxx] Constrain common_iterator's iterator_traits specialization

[Casey Carter via Phabricator via libcxx-commits]

CaseyCarter added a comment.

> I don't object to landing this as-is, just to fix the bug; I would also be happy to merge it into D117400 <https://reviews.llvm.org/D117400>. Your call.

I have a mild preference to merge this small fix as-is.



================
Comment at: libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp:55
+    static_assert(!HasIteratorConcept<IterTraits>);
+    static_assert(std::same_as<IterTraits::iterator_category, std::output_iterator_tag>);
+    static_assert(std::same_as<IterTraits::value_type, void>);
----------------
ldionne wrote:
> Quuxplusone wrote:
> > Well this is weird. `non_const_deref_iterator<int*>::iterator_category` appears to be `input_iterator_tag`, but `std::common_iterator<Iter, sentinel_type<int*>>` is an output iterator instead?
> > 
> > Also, I'm seeing now that `non_const_deref_iterator` is a template, but it's used in only one place (with `int*`), so it can be massively simplified. Would you like me to just take this over and merge it into my existing D117400?
> Yeah, I'd like to understand this part as well.
`input_iterator` requires that `const` iterators are dereferenceable. Hopefully it's obvious that is not the case for `non_const_deref_iterator`, so it does not trigger the `iterator_traits<common_iterator<I, S>>` partial specialization once that partial specialization is properly constrained to require `input_iterator`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117449