Augment DynamicConverter's is_container check
authorNicholas Ormrod <njormrod@fb.com>
Tue, 6 Nov 2012 17:22:57 +0000 (09:22 -0800)
committerJordan DeLong <jdelong@fb.com>
Sun, 16 Dec 2012 22:43:10 +0000 (14:43 -0800)
Summary:
DynamicConverter uses some simple heuristics to determine if a
class is a container. One of those tests was to check that the
constructor 'template <class Iterator> [container-name](Iterator first,
Iterator last)' was present. That test was performed by checking if the
class could be constructed by two parameters of some dummy class.

However, it is possible to restrict the template parameter such that it
only accepts iterators, and not any arbitrary dummy class. This would be
useful, for example, to solve overload ambiguity with constructors like
'vector(const T& val, size_type n)', where T and size_type are the same
(see N3337 23.2.3 item 14). It also (I believe) produces more meaningful
compiler errors when a non-iterator is supplied, since it errors at the
function callsite instead of inside the constructor itself.

The new FBVector implementation uses such a feature, and so checking for
[container-name](dummy, dummy) will fail. Hence the dummy class has been
upgraded to reverse_iterator<T*>, a valid iterator class which almost
certainly does not have a specialized contructor in any class (and hence
will not cause any visible change in class_is_container's value).

Test Plan:
Run DynamicConverterTest; it has tests for the correctness of
class_is_container.

Reviewed By: delong.j@fb.com

FB internal diff: D620607

folly/DynamicConverter.h

index 2f0a1205888e9b15e1b1e98f61cca9abfb636add..b36a56cd8a1c9e1e1b90f4ab11723c4e90cf0bf3 100644 (file)
@@ -37,6 +37,7 @@ namespace folly {
 
 
 #include <type_traits>
+#include <iterator>
 #include <boost/iterator/iterator_adaptor.hpp>
 #include <boost/mpl/has_xxx.hpp>
 #include "folly/Likely.h"
@@ -59,10 +60,10 @@ template <typename T> struct map_container_has_correct_types
                  typename T::value_type> {};
 
 template <typename T> struct class_is_container {
-  struct dummy {};
+  typedef std::reverse_iterator<T*> some_iterator;
   enum { value = has_value_type<T>::value &&
                  has_iterator<T>::value &&
-                 std::is_constructible<T, dummy, dummy>::value };
+              std::is_constructible<T, some_iterator, some_iterator>::value };
 };
 
 template <typename T> struct container_is_map