[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;
> @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
CHANGES SINCE LAST ACTION
More information about the libcxx-commits