[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/memory:1726
+#else  // _LIBCPP_HAS_NO_VARIADICS
+    template <class _Tp, class _A0>
+        _LIBCPP_INLINE_VISIBILITY
----------------
vsapsai wrote:
> ldionne wrote:
> > Here,
> > 
> > ```
> > template <class _Tp, class _A0, class = typename enable_if<__has_construct<allocator_type, _Tp*, _A0 const&>::value>::type>
> > static void __construct(...);
> > ```
> > 
> > and then below
> > 
> > ```
> > template <class _Tp, class _A0, class = typename enable_if<!__has_construct<allocator_type, _Tp*, _A0 const&>::value>::type>
> > static void __construct(...);
> > ```
> > 
> > This way you don't have to call `__has_construct` at the point of call and `__construct` is slightly more reusable.
> > 
> I've tried that (also updated how `static void construct` calls `__construct`) and it works with C++2a but fails with C++03. The error is
> 
> ```
> llvm-project/libcxx/include/memory:1725:21: error: class member cannot be redeclared
>         static void __construct(allocator_type&, _Tp* __p, const _A0& __a0)
>                     ^
> llvm-project/libcxx/include/memory:1720:21: note: previous definition is here
>         static void __construct(allocator_type& __a, _Tp* __p, const _A0& __a0)
>                     ^
> ```
> 
> I haven't investigated further yet.
FWIW, I don't have a strong opinion here. I think Volodymyr's way instantiates slightly fewer intermediate types. It's also consistent with the definition of `__destroy` right below it. I don't know who we expect to be "reusing" `allocator_traits<A>::__construct`.

OTOH, in this case I don't see any concrete benefit to making the dispatch explicit at the callsite. (Whereas in D49317 (Ctrl+F "Got it") I needed the dispatch to be explicit at the callsite.) So refactoring `__construct` would be reasonable, although perhaps slower, and open a can of worms as to whether `__destroy` should also be refactored.




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

https://reviews.llvm.org/D48753