[libcxx-commits] [PATCH] D97862: [libc++] Introduce __identity_t<T>

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 4 04:49:40 PST 2021

ldionne added inline comments.

Comment at: libcxx/include/map:908
+    typedef __identity_t<_Compare>                   key_compare;
+    typedef __identity_t<_Allocator>                 allocator_type;
     typedef value_type&                              reference;
Quuxplusone wrote:
> @ldionne wrote:
> > This is non-blocking, however I think it might be nicer to use `__identity_t` on the arguments of the constructors that need them instead of on the typedefs. Otherwise, it's really unclear //why// this convolution is needed.
> I agree, but when I started trying to do that, I immediately ran into existing implementation divergence; see https://godbolt.org/z/bxhn84 (libstdc++ rejects, libc++ and MSVC accept) and my tonight's LWG reflector thread "P1518 redux: CTAD deducing Allocator/Compare from random constructors". I definitely think the libc++/MSVC behavior is the "correct" behavior, so I'd want to preserve it, even if it means putting `__identity_t` on more ctor arguments than the paper standard currently mandates.
> But now that's a feature-ish enough change that I'm gonna just land this "NFCI" commit as-is, and open a different PR if I decide it's worth doing anything else (which I still might, but it's no longer a slam-dunk).
Okay, I agree with that. Thanks for keeping changes small and separated!

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list