From: Giuseppe Ottaviano Date: Wed, 26 Oct 2016 23:47:00 +0000 (-0700) Subject: Add default constructor to dynamic, make other constructors simpler and stricter X-Git-Tag: v2016.10.31.00~4 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=94e964976c1b65e79b0f7bdd960cfec260399a52;p=folly.git Add default constructor to dynamic, make other constructors simpler and stricter Summary: Now that the initializer list syntax has been removed we can add a default constructor. Also, - The `dynamic(T)` constructor was unconstrained, so it would match any type but then fail to compile (as a side effect, `is_convertible` would be always true). This also leaked the implementation details of `Array` and `Object`, as they were accepted as arguments. The diff makes the constructor accept only integral and float arguments, and all other types are SFINAEd out. - `dynamic(Iterator, Iterator)` is made `explicit` to avoid accepting statements like `dynamic d = {"a", "b"};`. - `object(...)` methods are simplified. Reviewed By: luciang Differential Revision: D4065021 fbshipit-source-id: ac289da7bece67c674b7036b7b51d5e016b297e5 --- diff --git a/folly/docs/Dynamic.md b/folly/docs/Dynamic.md index 3d321022..5cf5ed2c 100644 --- a/folly/docs/Dynamic.md +++ b/folly/docs/Dynamic.md @@ -25,7 +25,7 @@ folly::dynamic;` was used): dynamic nul = nullptr; dynamic boolean = false; - // Arrays can be initialized with brackets. + // Arrays can be initialized with dynamic::array. dynamic array = dynamic::array("array ", "of ", 4, " elements"); assert(array.size() == 4); dynamic emptyArray = dynamic::array; @@ -173,19 +173,6 @@ static types, etc). ### Some Design Rationale *** -**Q. Why is there no default constructor?** - -This is a bit of a limitation of `std::initializer_list<>` for -this use case. The expression `dynamic d = {}` is required by the -standard to call the default constructor if one exists (the -reasoning for this makes sense, since `{}` is part of the concept -of "uniform initialization", and is intended for use with things -like `std::vector`). It would be surprising if this expression -didn't leave `d.isArray()` true, but on the other hand it would -also be surprising if `dynamic d` left `d.isArray()` as true. The -solution was just to disallow uninitialized dynamics: every -dynamic must start out being assigned to some value (or nullptr). - **Q. Why doesn't a dynamic string support begin(), end(), and operator[]?** The value_type of a dynamic iterator is `dynamic`, and `operator[]` diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h index a32da925..c1be95b9 100644 --- a/folly/dynamic-inl.h +++ b/folly/dynamic-inl.h @@ -92,44 +92,6 @@ namespace detail { template static void destroy(T* t) { t->~T(); } }; - /* - * The enable_if junk here is necessary to avoid ambiguous - * conversions relating to bool and double when you implicitly - * convert an int or long to a dynamic. - */ - template struct ConversionHelper; - template - struct ConversionHelper< - T, - typename std::enable_if< - std::is_integral::value && !std::is_same::value - >::type - > { - typedef int64_t type; - }; - template <> - struct ConversionHelper { - typedef double type; - }; - template - struct ConversionHelper< - T, - typename std::enable_if< - (!std::is_integral::value || std::is_same::value) && - !std::is_same::value && - !std::is_same::value>::type> { - typedef T type; - }; - template - struct ConversionHelper< - T, - typename std::enable_if< - std::is_same::value - >::type - > { - typedef void* type; - }; - /* * Helper for implementing numeric conversions in operators on * numbers. Just promotes to double when one of the arguments is @@ -173,12 +135,7 @@ struct dynamic::ObjectMaker { friend struct dynamic; explicit ObjectMaker() : val_(dynamic::object) {} - explicit ObjectMaker(dynamic const& key, dynamic val) - : val_(dynamic::object) - { - val_.insert(key, std::move(val)); - } - explicit ObjectMaker(dynamic&& key, dynamic val) + explicit ObjectMaker(dynamic key, dynamic val) : val_(dynamic::object) { val_.insert(std::move(key), std::move(val)); @@ -191,15 +148,10 @@ struct dynamic::ObjectMaker { ObjectMaker& operator=(ObjectMaker const&) = delete; ObjectMaker& operator=(ObjectMaker&&) = delete; - // These return rvalue-references instead of lvalue-references to allow - // constructs like this to moved instead of copied: + // This returns an rvalue-reference instead of an lvalue-reference + // to allow constructs like this to moved instead of copied: // dynamic a = dynamic::object("a", "b")("c", "d") - ObjectMaker&& operator()(dynamic const& key, dynamic val) { - val_.insert(key, std::move(val)); - return std::move(*this); - } - - ObjectMaker&& operator()(dynamic&& key, dynamic val) { + ObjectMaker&& operator()(dynamic key, dynamic val) { val_.insert(std::move(key), std::move(val)); return std::move(*this); } @@ -215,26 +167,10 @@ inline dynamic dynamic::array(Args&& ...args) { return dynamic(Array{std::forward(args)...}); } -// This looks like a case for perfect forwarding, but our use of -// std::initializer_list for constructing dynamic arrays makes it less -// functional than doing this manually. - -// TODO(ott, 10300209): When the initializer_list constructor is gone, -// simplify this. inline dynamic::ObjectMaker dynamic::object() { return ObjectMaker(); } -inline dynamic::ObjectMaker dynamic::object(dynamic&& a, dynamic&& b) { +inline dynamic::ObjectMaker dynamic::object(dynamic a, dynamic b) { return ObjectMaker(std::move(a), std::move(b)); } -inline dynamic::ObjectMaker dynamic::object(dynamic const& a, dynamic&& b) { - return ObjectMaker(a, std::move(b)); -} -inline dynamic::ObjectMaker dynamic::object(dynamic&& a, dynamic const& b) { - return ObjectMaker(std::move(a), b); -} -inline dynamic::ObjectMaker -dynamic::object(dynamic const& a, dynamic const& b) { - return ObjectMaker(a, b); -} ////////////////////////////////////////////////////////////////////// @@ -275,6 +211,10 @@ struct dynamic::const_value_iterator ////////////////////////////////////////////////////////////////////// +inline dynamic::dynamic() : dynamic(nullptr) {} + +inline dynamic::dynamic(std::nullptr_t) : type_(NULLT) {} + inline dynamic::dynamic(void (*)(EmptyArrayTag)) : type_(ARRAY) { @@ -299,13 +239,9 @@ inline dynamic::dynamic(char const* s) new (&u_.string) std::string(s); } -inline dynamic::dynamic(std::string const& s) +inline dynamic::dynamic(std::string s) : type_(STRING) { - new (&u_.string) std::string(s); -} - -inline dynamic::dynamic(std::string&& s) : type_(STRING) { new (&u_.string) std::string(std::move(s)); } @@ -330,14 +266,32 @@ inline dynamic::dynamic(dynamic&& o) noexcept inline dynamic::~dynamic() noexcept { destroy(); } -template +// Integral types except bool convert to int64_t, float types to double. +template +struct dynamic::NumericTypeHelper< + T, typename std::enable_if::value>::type> { + using type = int64_t; +}; +template <> +struct dynamic::NumericTypeHelper { + using type = bool; +}; +template <> +struct dynamic::NumericTypeHelper { + using type = double; +}; +template <> +struct dynamic::NumericTypeHelper { + using type = double; +}; + +template::type */> dynamic::dynamic(T t) { - typedef typename detail::ConversionHelper::type U; - type_ = TypeInfo::type; - new (getAddress()) U(std::move(t)); + type_ = TypeInfo::type; + new (getAddress()) NumericType(t); } -template +template dynamic::dynamic(Iterator first, Iterator last) : type_(ARRAY) { @@ -679,7 +633,7 @@ inline dynamic::dynamic(Array&& r) : type_(ARRAY) { FOLLY_DYNAMIC_DEC_TYPEINFO(void*, "null", dynamic::NULLT) FOLLY_DYNAMIC_DEC_TYPEINFO(bool, "boolean", dynamic::BOOL) -FOLLY_DYNAMIC_DEC_TYPEINFO(std::string, "string", dynamic::STRING) +FOLLY_DYNAMIC_DEC_TYPEINFO(std::string, "string", dynamic::STRING) FOLLY_DYNAMIC_DEC_TYPEINFO(dynamic::Array, "array", dynamic::ARRAY) FOLLY_DYNAMIC_DEC_TYPEINFO(double, "double", dynamic::DOUBLE) FOLLY_DYNAMIC_DEC_TYPEINFO(int64_t, "int64", dynamic::INT64) diff --git a/folly/dynamic.h b/folly/dynamic.h index 2dc49434..c6890e4e 100644 --- a/folly/dynamic.h +++ b/folly/dynamic.h @@ -45,16 +45,6 @@ * Also see folly/json.h for the serialization and deserialization * functions for JSON. * - * Note: dynamic is not DefaultConstructible. Rationale: - * - * - The intuitive thing to initialize a defaulted dynamic to would - * be nullptr. - * - * - However, the expression dynamic d = {} is required to call the - * default constructor by the standard, which is confusing - * behavior for dynamic unless the default constructor creates an - * empty array. - * * Additional documentation is in folly/docs/Dynamic.md. * * @author Jordan DeLong @@ -95,6 +85,7 @@ struct dynamic : private boost::operators { OBJECT, STRING, }; + template struct NumericTypeHelper; /* * We support direct iteration of arrays, and indirect iteration of objects. @@ -143,18 +134,20 @@ public: static dynamic array(Args&& ...args); static ObjectMaker object(); - static ObjectMaker object(dynamic&&, dynamic&&); - static ObjectMaker object(dynamic const&, dynamic&&); - static ObjectMaker object(dynamic&&, dynamic const&); - static ObjectMaker object(dynamic const&, dynamic const&); + static ObjectMaker object(dynamic, dynamic); + + /** + * Default constructor, initializes with nullptr. + */ + dynamic(); /* * String compatibility constructors. */ + /* implicit */ dynamic(std::nullptr_t); /* implicit */ dynamic(StringPiece val); /* implicit */ dynamic(char const* val); - /* implicit */ dynamic(std::string const& val); - /* implicit */ dynamic(std::string&& val); + /* implicit */ dynamic(std::string val); /* * This is part of the plumbing for array() and object(), above. @@ -166,15 +159,18 @@ public: /* implicit */ dynamic(ObjectMaker&&); /* - * Conversion constructors from most of the other types. + * Constructors for integral and float types. + * Other types are SFINAEd out with NumericTypeHelper. */ - template /* implicit */ dynamic(T t); + template::type> + /* implicit */ dynamic(T t); /* * Create a dynamic that is an array of the values from the supplied * iterator range. */ - template dynamic(Iterator first, Iterator last); + template + explicit dynamic(Iterator first, Iterator last); dynamic(dynamic const&); dynamic(dynamic&&) noexcept; diff --git a/folly/test/DynamicTest.cpp b/folly/test/DynamicTest.cpp index 27d51dc6..c19d2625 100644 --- a/folly/test/DynamicTest.cpp +++ b/folly/test/DynamicTest.cpp @@ -31,6 +31,11 @@ void dynamic::print_as_pseudo_json(std::ostream& out) const { out << ""; } +TEST(Dynamic, Default) { + dynamic obj; + EXPECT_TRUE(obj.isNull()); +} + TEST(Dynamic, ObjectBasics) { dynamic obj = dynamic::object("a", false); EXPECT_EQ(obj.at("a"), false);