vsapsai added inline comments.

================
Comment at: libcxx/include/memory:1470
+        decltype(_VSTD::declval<_Alloc>().construct(_VSTD::declval<_Pointer>(),
+                                                    _VSTD::declval<_Args>())),
+        void
----------------
Quuxplusone wrote:
> vsapsai wrote:
> > Quuxplusone wrote:
> > > I think you should replace this `)))` with `)), void())` for absolute 
> > > correctness (e.g. `construct` might plausibly return a pointer to the 
> > > constructed object, and I think C++03 is okay with that).
> > > Otherwise, awesome, this looks like what I'd expect. :)
> > The standard says the result of `construct` is not used but I haven't found 
> > any mention it **must** be `void`.
> > 
> > That being said, the proposed change fails for the C++03 macro-based 
> > decltype shim:
> > 
> >     error: too many arguments provided to function-like macro invocation
> >                                         _VSTD::declval<_Args>()), void()),
> >                                                                   ^
> > 
> > One option to fix that is to use
> > 
> >     is_same
> >     <
> >         decltype(_as_before_),
> >         decltype(_the_same_copy_pasted_argument_)
> >     >
> > 
> > But I don't think this use case is worth such hacks.
> I think the workaround is easier. Try
> 
> ```
> template <class _Alloc, class _Pointer, class _A0>
> struct __has_construct<_Alloc, _Pointer, _A0, typename enable_if<
>   is_same<
>     decltype((_VSTD::declval<_Alloc>().construct(
>       _VSTD::declval<_Pointer>(),
>       _VSTD::declval<_A0>()
>     ), void() )),
>     void
>   >::value
> >::type> : std::true_type {};
> ```
> 
> i.e. with an extra set of parens to group the argument to the `decltype` 
> macro: `decltype(())` instead of `decltype()`. I also drive-by edited `Args` 
> to `A0`, matching how it's done elsewhere in pseudo-variadic-templates in 
> this file.
Thanks for the suggestion, Arthur, that worked.

Regarding `_Args` vs `_A0`, I am deliberately using `_Args` because it's not a 
manual pseudo-variadic-template expansion and it's not inside 
`_LIBCPP_HAS_NO_VARIADICS` block. But I don't feel strong about it, can change 
to `_A0`.


https://reviews.llvm.org/D48753



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to