dlj created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.
Some container operations require ADL. For example, std::advance is
required to use specific operators, which will participate in ADL.
However, implementation details which rely on SFINAE should be careful not to
inadvertently invoke ADL. Otherwise, the SFINAE lookup will (incorrectly)
consider namespaces other than std::. This is particularly problematic with
incomplete types, since the set of associated namespaces for a class includes
the body of the class (which may contain inline friend function overloads). If a
type is incomplete, its body cannot be added to the set of associated
namespaces; this results in an error.
The changes in this patch mostly appear to be omissions. Several of the changes
are for SFINAE overloads with internal names (in some cases, there are other
uses which are already correctly qualified). In a few cases, the implementation
details of iterators are directly to avoid invoking ADL (this seems unfortunate;
but on balance, better than failing when type erasure is otherwise plausible).
https://reviews.llvm.org/D37538
Files:
include/__split_buffer
include/deque
include/memory
test/std/containers/containers.general/construct_destruct.pass.cpp
Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===================================================================
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,130 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <array>
+#include <cassert>
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <queue>
+#include <set>
+#include <stack>
+#include <type_traits>
+#include <unordered_map>
+#include <unordered_set>
+#include <vector>
+
+// Holder<ns::T> is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder<ns::T> *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+// namespace ns {
+// class T {
+// // 13.5.7 [over.inc]:
+// friend std::list<T *>::const_iterator
+// operator++(std::list<T *>::const_iterator it) {
+// return /* ... */;
+// }
+// };
+// }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+// // Defined in a header file:
+// class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector<Impl*> impls;
+// public:
+// ~InterfaceList();
+// };
+//
+template <class T>
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template <template <class...> class Container>
+void TestSequencePtr() {
+ using X = Container<Holder<ns::Fwd>*>;
+ {
+ X u;
+ assert(u.empty());
+ }
+ {
+ auto u = X();
+ assert(u.empty());
+ }
+}
+
+template <template <class...> class Container>
+void TestMappingPtr() {
+ {
+ using X = Container<int, Holder<ns::Fwd>*>;
+ X u1;
+ assert(u1.empty());
+ auto u2 = X();
+ assert(u2.empty());
+ }
+ {
+ using X = Container<Holder<ns::Fwd>*, int>;
+ X u1;
+ assert(u1.empty());
+ auto u2 = X();
+ assert(u2.empty());
+ }
+}
+
+int main()
+{
+ // Sequence containers:
+ {
+ std::array<Holder<ns::Fwd>*, 1> a;
+ (void)a;
+ }
+ TestSequencePtr<std::deque>();
+ TestSequencePtr<std::forward_list>();
+ TestSequencePtr<std::list>();
+ TestSequencePtr<std::vector>();
+
+ // Associative containers, non-mapping:
+ TestSequencePtr<std::set>();
+ TestSequencePtr<std::unordered_set>();
+
+ // Associative containers, mapping:
+ TestMappingPtr<std::map>();
+ TestMappingPtr<std::unordered_map>();
+
+ // Container adapters:
+ TestSequencePtr<std::queue>();
+ TestSequencePtr<std::stack>();
+}
Index: include/memory
===================================================================
--- include/memory
+++ include/memory
@@ -1346,9 +1346,9 @@
struct __has_construct
: integral_constant<bool,
is_same<
- decltype(__has_construct_test(declval<_Alloc>(),
- declval<_Pointer>(),
- declval<_Args>()...)),
+ decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
+ declval<_Args>()...)),
true_type>::value>
{
};
@@ -1367,8 +1367,8 @@
struct __has_destroy
: integral_constant<bool,
is_same<
- decltype(__has_destroy_test(declval<_Alloc>(),
- declval<_Pointer>())),
+ decltype(_VSTD::__has_destroy_test(declval<_Alloc>(),
+ declval<_Pointer>())),
true_type>::value>
{
};
@@ -1387,7 +1387,7 @@
struct __has_max_size
: integral_constant<bool,
is_same<
- decltype(__has_max_size_test(declval<_Alloc&>())),
+ decltype(_VSTD::__has_max_size_test(declval<_Alloc&>())),
true_type>::value>
{
};
@@ -1406,7 +1406,7 @@
struct __has_select_on_container_copy_construction
: integral_constant<bool,
is_same<
- decltype(__has_select_on_container_copy_construction_test(declval<_Alloc&>())),
+ decltype(_VSTD::__has_select_on_container_copy_construction_test(declval<_Alloc&>())),
true_type>::value>
{
};
@@ -1547,7 +1547,7 @@
template <class _Tp>
_LIBCPP_INLINE_VISIBILITY
static void destroy(allocator_type& __a, _Tp* __p)
- {__destroy(__has_destroy<allocator_type, _Tp*>(), __a, __p);}
+ {__destroy(_VSTD::__has_destroy<allocator_type, _Tp*>(), __a, __p);}
_LIBCPP_INLINE_VISIBILITY
static size_type max_size(const allocator_type& __a) _NOEXCEPT
Index: include/deque
===================================================================
--- include/deque
+++ include/deque
@@ -1164,8 +1164,8 @@
__deque_base<_Tp, _Allocator>::clear() _NOEXCEPT
{
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;
while (__map_.size() > 2)
{
Index: include/__split_buffer
===================================================================
--- include/__split_buffer
+++ include/__split_buffer
@@ -276,7 +276,7 @@
__split_buffer<_Tp, _Allocator>::__destruct_at_begin(pointer __new_begin, false_type)
{
while (__begin_ != __new_begin)
- __alloc_traits::destroy(__alloc(), __to_raw_pointer(__begin_++));
+ __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(__begin_++));
}
template <class _Tp, class _Allocator>
@@ -293,7 +293,7 @@
__split_buffer<_Tp, _Allocator>::__destruct_at_end(pointer __new_last, false_type) _NOEXCEPT
{
while (__new_last != __end_)
- __alloc_traits::destroy(__alloc(), __to_raw_pointer(--__end_));
+ __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
}
template <class _Tp, class _Allocator>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
