[libcxx-commits] [PATCH] D99863: [libcxx] adds `iter_difference_t` and `iter_value_t`

[Tim Song via Phabricator via libcxx-commits]

tcanens added inline comments.


================
Comment at: libcxx/include/iterator:486
+// `incrementable_­traits<RI>::difference_­type` if `iterator_­traits<RI>` names a specialization
+// generated from the primary template, and `iterator_­traits<R[I]>::difference_­type` otherwise.
+template <class _Ip>
----------------
This now has a mix of `RI` and `R[I]`


================
Comment at: libcxx/include/iterator:489
+    incrementable_traits<remove_cvref_t<_Ip>>,
+    iterator_traits<remove_cvref_t<_Ip>>>::difference_type;
 
----------------
cjdb wrote:
> Quuxplusone wrote:
> > Please write `RI`, not `R[I]`. (I know you're trying to reflect that the `I` in the standard is a subscript letter. However, it's not an array index, and `RI` functions semantically as a single identifier — there is no separate entity `R` to be discussed, so we don't need a name for it.)
> > 
> > I don't think you should be instantiating `incrementable_traits<RI>` in the case that `iterator_traits<RI>` is the primary template. Please (1) check what the Standard's intent here is, (2) make the fix if needed, and (3) add a test case that detects the fix.
> > 
> > Separately and presumably a moot point in this case: note that `conditional_t` is compile-time-wasting by design. Anywhere in libc++ that we need its effects and aren't mandated to use `conditional_t`, we should be using Eric's `_If<is-blah, incr-traits, iter-traits>` because it instantiates fewer templates.
> > I don't think you should be instantiating `incrementable_traits<RI>` in the case that `iterator_traits<RI>` is the primary template. Please (1) check what the Standard's intent here is, (2) make the fix if needed, and (3) add a test case that detects the fix.
> 
> It's pretty much left up to us. MSVC's implementation is identical to the `conditional_t` one, for example, and @CaseyCarter understands the intention better than us all.
> I don't think you should be instantiating `incrementable_traits<RI>` in the case that `iterator_traits<RI>` is the primary template. 

What does this even mean? How do you produce `incrementable_­traits<RI>::difference_­type` without instantiating `incrementable_­traits<RI>`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99863