erik.pilkington added inline comments.
================
Comment at: libcxx/include/memory:1479
+struct __has_construct_missing
+ : false_type
+{
----------------
vsapsai wrote:
> erik.pilkington wrote:
> > Shouldn't this be true_type?
> I see this is confusing and I'm still struggling how to express it. The issue
> is that in C++03 `__has_construct` should be something like unknown, so that
> neither `__has_construct` nor `! __has_construct` evaluate to true because we
> don't really know if allocator has construct. This case is covered by the
> added test, in C++03 the memcpy specialization was enabled when
>
> ```
> is_same<allocator_type, allocator<_Tp> >
> || !false_type
> ```
>
> So `is_same` check had no effect and we were using memcpy to convert between
> int and float.
>
> I was considering using something like
>
> ```lang=c++
> typename enable_if
> <
> (is_same
> <
> typename _VSTD::remove_const<typename
> allocator_type::value_type>::type,
> typename _VSTD::remove_const<_SourceTp>::type
> >::value
> #ifndef _LIBCPP_CXX03_LANG
> || !__has_construct<allocator_type, _DestTp*,
> _SourceTp>::value
> #endif
> ) &&
> is_trivially_move_constructible<_DestTp>::value,
> void
> >::type
> ```
>
> But that doesn't look readable to me, so I've introduced ad-hoc ternary logic
> with `__has_construct_missing`.
Oh I see, yikes! That's a pretty bad bug. I agree that this fix is best then,
but can you add a comment explaining this to `__has_construct_missing` for
future casual readers? Also, I think we should move the __has_construct_missing
bugfix into a different (prerequisite) patch. Seems unrelated to the `const`
related optimization below.
https://reviews.llvm.org/D48342
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
