Commit 6a7ee8aa authored by Nicholas Ormrod's avatar Nicholas Ormrod Committed by Jordan DeLong

range checks in DynamicConverter

Summary:
Add range-checking to convertTo for small numeric types.

Internally, dynamics represent a numeric with an int64_t or a double.
When converting to a smaller numeric type, DynamicConverter uses a
static_cast. This causes some confusion (re D936940). The code now uses
folly::to, which throws a std::range_error on overflow.

While working on this I also added some light comments to the new
toDynamic section, for consistency with the original convertTo
commenting. I also renamed the internal trait is_associative_container
to is_map, since is_associative_container is looking for a mapped_type
typedef and hence excludes such associative containers as sets.

While adding the overflow tests, I also augmented the typetraits test to
include the is_map and is_range traits, which hitherto had no test
coverage.

Test Plan: build and run tests, both in dbg and opt

Reviewed By: cberner@fb.com

FB internal diff: D961605
parent 5f19afe7
......@@ -81,7 +81,7 @@ template <typename T> struct is_range
std::false_type
>::type {};
template <typename T> struct is_associative_container
template <typename T> struct is_map
: std::integral_constant<
bool,
is_range<T>::value && has_mapped_type<T>::value
......@@ -178,13 +178,14 @@ conversionIterator(const It& it) {
///////////////////////////////////////////////////////////////////////////////
// DynamicConverter specializations
template <typename T, typename Enable = void> struct DynamicConverter;
/**
* Each specialization of DynamicConverter has the function
* 'static T convert(const dynamic& d);'
* 'static T convert(const dynamic&);'
*/
// default - intentionally unimplemented
template <typename T, typename Enable = void> struct DynamicConverter;
// boolean
template <>
struct DynamicConverter<bool> {
......@@ -199,7 +200,7 @@ struct DynamicConverter<T,
typename std::enable_if<std::is_integral<T>::value &&
!std::is_same<T, bool>::value>::type> {
static T convert(const dynamic& d) {
return static_cast<T>(d.asInt());
return folly::to<T>(d.asInt());
}
};
......@@ -208,7 +209,7 @@ template <typename T>
struct DynamicConverter<T,
typename std::enable_if<std::is_floating_point<T>::value>::type> {
static T convert(const dynamic& d) {
return static_cast<T>(d.asDouble());
return folly::to<T>(d.asDouble());
}
};
......@@ -261,9 +262,17 @@ struct DynamicConverter<C,
throw TypeError("object or array", d.type());
}
}
};
///////////////////////////////////////////////////////////////////////////////
// DynamicConstructor specializations
/**
* Each specialization of DynamicConstructor has the function
* 'static dynamic construct(const C&);'
*/
// default
template <typename C, typename Enable = void>
struct DynamicConstructor {
static dynamic construct(const C& x) {
......@@ -271,10 +280,11 @@ struct DynamicConstructor {
}
};
// maps
template<typename C>
struct DynamicConstructor<C,
typename std::enable_if<
dynamicconverter_detail::is_associative_container<C>::value>::type> {
dynamicconverter_detail::is_map<C>::value>::type> {
static dynamic construct(const C& x) {
dynamic d = dynamic::object;
for (auto& pair : x) {
......@@ -284,10 +294,11 @@ struct DynamicConstructor<C,
}
};
// other ranges
template<typename C>
struct DynamicConstructor<C,
typename std::enable_if<
!dynamicconverter_detail::is_associative_container<C>::value &&
!dynamicconverter_detail::is_map<C>::value &&
!std::is_constructible<StringPiece, const C&>::value &&
dynamicconverter_detail::is_range<C>::value>::type> {
static dynamic construct(const C& x) {
......@@ -299,6 +310,7 @@ struct DynamicConstructor<C,
}
};
// pair
template<typename A, typename B>
struct DynamicConstructor<std::pair<A, B>, void> {
static dynamic construct(const std::pair<A, B>& x) {
......@@ -310,7 +322,7 @@ struct DynamicConstructor<std::pair<A, B>, void> {
};
///////////////////////////////////////////////////////////////////////////////
// convertTo implementation
// implementation
template <typename T>
T convertTo(const dynamic& d) {
......
......@@ -46,6 +46,26 @@ TEST(DynamicConverter, template_metaprogramming) {
EXPECT_EQ(c1t, true);
EXPECT_EQ(c2t, true);
EXPECT_EQ(c3t, true);
bool m1f = is_map<int>::value;
bool m2f = is_map<std::set<int>>::value;
bool m1t = is_map<std::map<int, int>>::value;
EXPECT_EQ(m1f, false);
EXPECT_EQ(m2f, false);
EXPECT_EQ(m1t, true);
bool r1f = is_range<int>::value;
bool r1t = is_range<std::set<int>>::value;
bool r2t = is_range<std::vector<int>>::value;
EXPECT_EQ(r1f, false);
EXPECT_EQ(r1t, true);
EXPECT_EQ(r2t, true);
}
TEST(DynamicConverter, arithmetic_types) {
......@@ -57,10 +77,6 @@ TEST(DynamicConverter, arithmetic_types) {
auto i2 = convertTo<int64_t>(d2);
EXPECT_EQ(i2, 123456789012345);
dynamic d3 = 123456789012345;
auto i3 = convertTo<uint8_t>(d3);
EXPECT_EQ(i3, 121);
dynamic d4 = 3.141;
auto i4 = convertTo<float>(d4);
EXPECT_EQ((int)(i4*100), 314);
......@@ -332,6 +348,19 @@ TEST(DynamicConverter, construct) {
}
}
TEST(DynamicConverter, errors) {
const auto int32Over =
static_cast<int64_t>(std::numeric_limits<int32_t>().max()) + 1;
const auto floatOver =
static_cast<double>(std::numeric_limits<float>().max()) * 2;
dynamic d1 = int32Over;
EXPECT_THROW(convertTo<int32_t>(d1), std::range_error);
dynamic d2 = floatOver;
EXPECT_THROW(convertTo<float>(d2), std::range_error);
}
int main(int argc, char ** argv) {
testing::InitGoogleTest(&argc, argv);
google::ParseCommandLineFlags(&argc, &argv, true);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment