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

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter 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>;
+
----------------
Quuxplusone wrote:
> 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.
Ideally, `Same` would be implemented with an intrinsic that tells the compiler to ignore the parameter mappings (http://eel.is/c++draft/temp.constr.atomic#2) when it checks the atomic constraints defined in this concept for equivalence. Or better yet, we could expose that feature in the language - say with a context-sensitive keyword `concept symmetric Same` - that could be applied to any two-type concept. There are quite a few symmetric concepts that could benefit similarly - `Common`, `CommonReference`, all of the `FooWith` concepts - for which we don't require subsumption because evaluating them is significantly more expensive than `Same`.


================
Comment at: include/concepts:152
+#if !defined(_LIBCPP_HAS_NO_CONCEPTS) && _LIBCPP_STD_VER > 17
+#define __cpp_lib_concepts 201806L
+
----------------
I've just realized I failed to update `<version>` with the feature test macro value. Does libc++ have a plan yet for how to handle the feature test macros? Should I `#define` it in `<version>` and include that here?

I suspect we should rename `__config` to `<version>` and define all feature test macros therein.


https://reviews.llvm.org/D49120