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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/iterator:489
+    incrementable_traits<remove_cvref_t<_Ip>>,
+    iterator_traits<remove_cvref_t<_Ip>>>::difference_type;
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99863