[PATCH] D49120: [libc++] P0898R3 2 of 12: Implement <concepts> header

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

Quuxplusone added inline comments.


================
Comment at: include/concepts:175
+template <class _Tp, class _Up>
+_LIBCPP_CONCEPT_DECL Same = __same_impl<_Tp, _Up> && __same_impl<_Up, _Tp>;
+
----------------
CaseyCarter wrote:
> Quuxplusone wrote:
> > Peanut gallery asks: From lines 166-171 it looks awfully like `__same_impl<_Tp, _Up>` is true if and only if `__same_impl<_Up, _Tp>` is true. So why bother instantiating both templates?
> This is the library implementation of the ["`Same<T, U>` subsumes `Same<U, T>`" requirement](http://eel.is/c++draft/concept.same#1).
Ah, so this and tangentially line 280 are both about fiddling with "subsumption." Admittedly I don't know how the compiler side is implemented, but it seems too bad that the library has to do double the work (even at this leafy of a level) just to deal with subsumption, which AIUI is required in order to properly order competing template specializations (a situation that is(?) unlikely to come up in the average end-user's code). :/
Anyway, carry on.


================
Comment at: include/concepts:394
+    CommonReference<
+        const remove_reference_t<_Tp>&,
+        const remove_reference_t<_Up>&> &&
----------------
CaseyCarter wrote:
> Quuxplusone wrote:
> > I'm fairly confident that `const remove_reference_t<_Tp>&` is just `const _Tp&`.
> Not when `_Tp` is a reference type: https://godbolt.org/g/Q2Hon8.
Ah, right, because of the `const`. Never mind!


https://reviews.llvm.org/D49120