EricWF added inline comments.
================
Comment at: include/__split_buffer:279
while (__begin_ != __new_begin)
- __alloc_traits::destroy(__alloc(), __to_raw_pointer(__begin_++));
+ __alloc_traits::destroy(__alloc(),
_VSTD::__to_raw_pointer(__begin_++));
}
----------------
The changes adding `_VSTD::` LGTM.
================
Comment at: include/deque:1167-1168
allocator_type& __a = __alloc();
- for (iterator __i = begin(), __e = end(); __i != __e; ++__i)
- __alloc_traits::destroy(__a, _VSTD::addressof(*__i));
+ for (iterator __i = begin(), __e = end(); __i.__ptr_ != __e.__ptr_;
__i.operator++())
+ __alloc_traits::destroy(__a, _VSTD::addressof(__i.operator*()));
size() = 0;
----------------
rsmith wrote:
> The other changes all look like obvious improvements to me. This one is a
> little more subtle, but if we want types like `deque<Holder<Incomplete> *>`
> to be destructible, I think we need to do something equivalent to this.
That's really yucky! I'm OK with this, but I really don't like it.
Fundamentally this can't work, at least not generically. When the allocator
produces a fancy pointer type, the operator lookups need to be performed using
ADL.
In this specific case, we control the iterator type, but this isn't always
true. for example like in `__split_buffer`, which uses the allocators pointer
type as an iterator.
@dlj I updated your test case to demonstrate the fancy-pointer problem:
https://gist.github.com/EricWF/b465fc475f55561ba972b6dd87e7e7ea
So I think we have a couple choices:
(1) Do nothing, since we can't universally support the behavior, so we
shouldn't attempt to support any form of this behavior.
(2) Fix only the non-fancy pointer case (which is what this patch does).
(3) Attempt to fix *all* cases, including the fancy-pointer ones. This won't be
possible, but perhaps certain containers can tolerate incomplete fancy pointers?
Personally I'm leaning towards (1), since we can't claim to support the use
case, at least not universally. If we select (1) we should probably encode the
restriction formally in standardese.
================
Comment at: include/memory:1349
is_same<
- decltype(__has_construct_test(declval<_Alloc>(),
- declval<_Pointer>(),
- declval<_Args>()...)),
+ decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
----------------
Wouldn't the `declval` calls also need qualification?
https://reviews.llvm.org/D37538
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
