[libcxx-commits] [PATCH] D116538: [libc++][P2321R2] Add specializations of basic_common_reference and common_type for tuple

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/common_reference.compile.pass.cpp:54-63
 namespace std {
 template <>
 struct common_type<X2, Y2> {
   using type = Z2;
 };
 template <>
 struct common_type<Y2, X2> {
----------------
Quuxplusone wrote:
> philnik wrote:
> > Quuxplusone wrote:
> > > While you're here, I suggest rewriting lines 54–63 to not reopen namespace std:
> > > ```
> > > template<> struct std::common_type<X2, Y2> { using type = Z2; }
> > > template<> struct std::common_type<Y2, X2> { using type = Z2; }
> > > ```
> > > And please add some tests involving them, e.g. IIUC
> > > ```
> > > static_assert(std::is_same_v<std::common_reference_t<std::tuple<int, X2>, std::tuple<int, Y2>>, std::tuple<int, Z2>>);
> > > static_assert(std::is_same_v<std::common_reference_t<std::tuple<int, X2>, std::tuple<int, Y2>>, std::tuple<int, Z2>>);
> > > static_assert(!has_type<std::common_reference<std::tuple<int, const X2>, std::tuple<float, const Y2>>);
> > > static_assert(!has_type<std::common_reference<std::tuple<int, X2>, std::tuple<float, Y2>>);
> > > static_assert(!has_type<std::common_reference<std::tuple<int, X2>, int, X2>);
> > > ```
> > > or whatever the results are supposed to be.
> > > 
> > > I haven't thought much about this, but it would probably be good to add a couple more specializations involving `tuple` to this test, e.g.
> > > ```
> > > template<> struct std::common_type<X3, std::tuple<X2>> { using type = Z3; }
> > > template<> struct std::common_type<std::tuple<X2>, X3> { using type = Z3; }
> > > template<> struct std::common_type<X3, std::tuple<Y2>> { using type = Z3; }
> > > template<> struct std::common_type<std::tuple<Y2>, X3> { using type = Z3; }
> > > ```
> > > and then test e.g. `common_type_t<std::tuple<X2>, std::tuple<Y2>, X3>` and `common_type_t<std::tuple<X2>, std::tuple<Y2>, std::tuple<X3>>`.
> > I'll refactor lines 54-63 in another patch.
> > I understand what you want to achieve with the common_reference asserts, but I don't know your intention with the common_type specializations.
> > I'll refactor lines 54-63 in another patch.
> 
> OK. Please also hit line 37 in that same patch.
> 
> > I understand what you want to achieve with the common_reference asserts, but I don't know your intention with the common_type specializations.
> 
> I don't know much about the mazey relationships between `common_type`, `common_reference`, `basic_common_reference`, so I might be asking for nonsense. :) Also I might have said "test `common_type_t<...`" when I meant "test `common_reference_t<...`"?  But basically I'd like to see
> - a test of variadic `common_{type,reference}_t<A,B,C>` where `common_type<A,B>` is due to the C++23-library-provided tuple specialization and `common_type<B,C>` is due to a user specialization
> - a user specialization of `common_type` that involves `std::tuple<SomeUserDefinedType>`, proving that the C++23-library-provided partial specialization isn't interfering-with or ambiguating that user specialization
> 
> Does that make more sense?
Yep, makes much more sense! But I think the two things are impossible to test separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116538