Commit 117385fb authored by Nick Terrell's avatar Nick Terrell Committed by facebook-github-bot-4

Overload on dynamic object reference type.

Summary: There are a bunch of methods in `folly::dynamic` that return a
reference to a sub-object, e.g. `getString()`.
These methods are a bit unsafe because it allows you to write code with dangling
references when the `folly::dynamic` object is an rvalue:

  auto& obj = parse_json(some_string).at("key");

However, the bigger win is that when we have an rvalue `folly::dynamic`, such as
returned by `getDefault(...)`, we can move the sub-object out:

  auto obj = parse_json(some_string).at("key");
  auto str = obj.getDefault("another-key", "").getString();

In the first line, the subobject is copied out when it could be safely moved out.
In the second line `getDefault(...)` copies the dynamic sub-object out, and then
`getString()` copies the string out, when the string could be moved.

Also in the case of `getDefault()` being called on a rvalue, we can move the
sub-object out.

Reviewed By: @marcinpe

Differential Revision: D2267588
parent d615b776
...@@ -384,15 +384,20 @@ inline double dynamic::asDouble() const { return asImpl<double>(); } ...@@ -384,15 +384,20 @@ inline double dynamic::asDouble() const { return asImpl<double>(); }
inline int64_t dynamic::asInt() const { return asImpl<int64_t>(); } inline int64_t dynamic::asInt() const { return asImpl<int64_t>(); }
inline bool dynamic::asBool() const { return asImpl<bool>(); } inline bool dynamic::asBool() const { return asImpl<bool>(); }
inline const fbstring& dynamic::getString() const { return get<fbstring>(); } inline const fbstring& dynamic::getString() const& { return get<fbstring>(); }
inline double dynamic::getDouble() const { return get<double>(); } inline double dynamic::getDouble() const& { return get<double>(); }
inline int64_t dynamic::getInt() const { return get<int64_t>(); } inline int64_t dynamic::getInt() const& { return get<int64_t>(); }
inline bool dynamic::getBool() const { return get<bool>(); } inline bool dynamic::getBool() const& { return get<bool>(); }
inline fbstring& dynamic::getString() { return get<fbstring>(); } inline fbstring& dynamic::getString() & { return get<fbstring>(); }
inline double& dynamic::getDouble() { return get<double>(); } inline double& dynamic::getDouble() & { return get<double>(); }
inline int64_t& dynamic::getInt() { return get<int64_t>(); } inline int64_t& dynamic::getInt() & { return get<int64_t>(); }
inline bool& dynamic::getBool() { return get<bool>(); } inline bool& dynamic::getBool() & { return get<bool>(); }
inline fbstring dynamic::getString() && { return std::move(get<fbstring>()); }
inline double dynamic::getDouble() && { return get<double>(); }
inline int64_t dynamic::getInt() && { return get<int64_t>(); }
inline bool dynamic::getBool() && { return get<bool>(); }
inline const char* dynamic::data() const& { return get<fbstring>().data(); } inline const char* dynamic::data() const& { return get<fbstring>().data(); }
inline const char* dynamic::c_str() const& { return get<fbstring>().c_str(); } inline const char* dynamic::c_str() const& { return get<fbstring>().c_str(); }
...@@ -460,10 +465,14 @@ inline dynamic& dynamic::operator--() { ...@@ -460,10 +465,14 @@ inline dynamic& dynamic::operator--() {
return *this; return *this;
} }
inline dynamic const& dynamic::operator[](dynamic const& idx) const { inline dynamic const& dynamic::operator[](dynamic const& idx) const& {
return at(idx); return at(idx);
} }
inline dynamic dynamic::operator[](dynamic const& idx) && {
return std::move((*this)[idx]);
}
template<class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) { template<class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) {
auto& obj = get<ObjectImpl>(); auto& obj = get<ObjectImpl>();
return obj.insert(std::make_pair(std::forward<K>(k), return obj.insert(std::make_pair(std::forward<K>(k),
...@@ -474,10 +483,14 @@ inline dynamic* dynamic::get_ptr(dynamic const& idx) & { ...@@ -474,10 +483,14 @@ inline dynamic* dynamic::get_ptr(dynamic const& idx) & {
return const_cast<dynamic*>(const_cast<dynamic const*>(this)->get_ptr(idx)); return const_cast<dynamic*>(const_cast<dynamic const*>(this)->get_ptr(idx));
} }
inline dynamic& dynamic::at(dynamic const& idx) { inline dynamic& dynamic::at(dynamic const& idx) & {
return const_cast<dynamic&>(const_cast<dynamic const*>(this)->at(idx)); return const_cast<dynamic&>(const_cast<dynamic const*>(this)->at(idx));
} }
inline dynamic dynamic::at(dynamic const& idx) && {
return std::move(at(idx));
}
inline bool dynamic::empty() const { inline bool dynamic::empty() const {
if (isNull()) { if (isNull()) {
return true; return true;
......
...@@ -132,7 +132,7 @@ dynamic& dynamic::operator=(dynamic&& o) noexcept { ...@@ -132,7 +132,7 @@ dynamic& dynamic::operator=(dynamic&& o) noexcept {
return *this; return *this;
} }
dynamic& dynamic::operator[](dynamic const& k) { dynamic& dynamic::operator[](dynamic const& k) & {
if (!isObject() && !isArray()) { if (!isObject() && !isArray()) {
throw TypeError("object/array", type()); throw TypeError("object/array", type());
} }
...@@ -144,20 +144,38 @@ dynamic& dynamic::operator[](dynamic const& k) { ...@@ -144,20 +144,38 @@ dynamic& dynamic::operator[](dynamic const& k) {
return ret.first->second; return ret.first->second;
} }
dynamic dynamic::getDefault(const dynamic& k, const dynamic& v) const { dynamic dynamic::getDefault(const dynamic& k, const dynamic& v) const& {
auto& obj = get<ObjectImpl>(); auto& obj = get<ObjectImpl>();
auto it = obj.find(k); auto it = obj.find(k);
return it == obj.end() ? v : it->second; return it == obj.end() ? v : it->second;
} }
dynamic&& dynamic::getDefault(const dynamic& k, dynamic&& v) const { dynamic dynamic::getDefault(const dynamic& k, dynamic&& v) const& {
auto& obj = get<ObjectImpl>(); auto& obj = get<ObjectImpl>();
auto it = obj.find(k); auto it = obj.find(k);
if (it != obj.end()) { // Avoid clang bug with ternary
v = it->second; if (it == obj.end()) {
return std::move(v);
} else {
return it->second;
} }
}
return std::move(v); dynamic dynamic::getDefault(const dynamic& k, const dynamic& v) && {
auto& obj = get<ObjectImpl>();
auto it = obj.find(k);
// Avoid clang bug with ternary
if (it == obj.end()) {
return v;
} else {
return std::move(it->second);
}
}
dynamic dynamic::getDefault(const dynamic& k, dynamic&& v) && {
auto& obj = get<ObjectImpl>();
auto it = obj.find(k);
return std::move(it == obj.end() ? v : it->second);
} }
const dynamic* dynamic::get_ptr(dynamic const& idx) const& { const dynamic* dynamic::get_ptr(dynamic const& idx) const& {
...@@ -180,7 +198,7 @@ const dynamic* dynamic::get_ptr(dynamic const& idx) const& { ...@@ -180,7 +198,7 @@ const dynamic* dynamic::get_ptr(dynamic const& idx) const& {
} }
} }
dynamic const& dynamic::at(dynamic const& idx) const { dynamic const& dynamic::at(dynamic const& idx) const& {
if (auto* parray = get_nothrow<Array>()) { if (auto* parray = get_nothrow<Array>()) {
if (!idx.isInt()) { if (!idx.isInt()) {
throw TypeError("int64", idx.type()); throw TypeError("int64", idx.type());
......
...@@ -283,14 +283,18 @@ public: ...@@ -283,14 +283,18 @@ public:
* *
* These will throw a TypeError if the dynamic has a different type. * These will throw a TypeError if the dynamic has a different type.
*/ */
const fbstring& getString() const; const fbstring& getString() const&;
double getDouble() const; double getDouble() const&;
int64_t getInt() const; int64_t getInt() const&;
bool getBool() const; bool getBool() const&;
fbstring& getString(); fbstring& getString() &;
double& getDouble(); double& getDouble() &;
int64_t& getInt(); int64_t& getInt() &;
bool& getBool(); bool& getBool() &;
fbstring getString() &&;
double getDouble() &&;
int64_t getInt() &&;
bool getBool() &&;
/* /*
* It is occasionally useful to access a string's internal pointer * It is occasionally useful to access a string's internal pointer
...@@ -362,8 +366,9 @@ public: ...@@ -362,8 +366,9 @@ public:
* will throw a TypeError. Using an index that is out of range or * will throw a TypeError. Using an index that is out of range or
* object-element that's not present throws std::out_of_range. * object-element that's not present throws std::out_of_range.
*/ */
dynamic const& at(dynamic const&) const; dynamic const& at(dynamic const&) const&;
dynamic& at(dynamic const&); dynamic& at(dynamic const&) &;
dynamic at(dynamic const&) &&;
/* /*
* Like 'at', above, except it returns either a pointer to the contained * Like 'at', above, except it returns either a pointer to the contained
...@@ -392,8 +397,9 @@ public: ...@@ -392,8 +397,9 @@ public:
* *
* These functions do not invalidate iterators. * These functions do not invalidate iterators.
*/ */
dynamic& operator[](dynamic const&); dynamic& operator[](dynamic const&) &;
dynamic const& operator[](dynamic const&) const; dynamic const& operator[](dynamic const&) const&;
dynamic operator[](dynamic const&) &&;
/* /*
* Only defined for objects, throws TypeError otherwise. * Only defined for objects, throws TypeError otherwise.
...@@ -404,8 +410,10 @@ public: ...@@ -404,8 +410,10 @@ public:
* a reference to the existing value if present, the new value otherwise. * a reference to the existing value if present, the new value otherwise.
*/ */
dynamic dynamic
getDefault(const dynamic& k, const dynamic& v = dynamic::object) const; getDefault(const dynamic& k, const dynamic& v = dynamic::object) const&;
dynamic&& getDefault(const dynamic& k, dynamic&& v) const; dynamic getDefault(const dynamic& k, dynamic&& v) const&;
dynamic getDefault(const dynamic& k, const dynamic& v = dynamic::object) &&;
dynamic getDefault(const dynamic& k, dynamic&& v) &&;
template<class K, class V = dynamic> template<class K, class V = dynamic>
dynamic& setDefault(K&& k, V&& v = dynamic::object); dynamic& setDefault(K&& k, V&& v = dynamic::object);
......
...@@ -308,6 +308,124 @@ TEST(Dynamic, Assignment) { ...@@ -308,6 +308,124 @@ TEST(Dynamic, Assignment) {
} }
} }
folly::fbstring make_long_string() {
return folly::fbstring(100, 'a');
}
TEST(Dynamic, GetDefault) {
const auto s = make_long_string();
dynamic ds(s);
dynamic tmp(s);
dynamic d1 = dynamic::object("key1", s);
dynamic d2 = dynamic::object("key2", s);
dynamic d3 = dynamic::object("key3", s);
dynamic d4 = dynamic::object("key4", s);
// lvalue - lvalue
dynamic ayy("ayy");
EXPECT_EQ(ds, d1.getDefault("key1", ayy));
EXPECT_EQ(ds, d1.getDefault("key1", ayy));
EXPECT_EQ(ds, d1.getDefault("not-a-key", tmp));
EXPECT_EQ(ds, tmp);
// lvalue - rvalue
EXPECT_EQ(ds, d1.getDefault("key1", "ayy"));
EXPECT_EQ(ds, d1.getDefault("key1", "ayy"));
EXPECT_EQ(ds, d1.getDefault("not-a-key", std::move(tmp)));
EXPECT_NE(ds, tmp);
// rvalue - lvalue
tmp = s;
EXPECT_EQ(ds, std::move(d1).getDefault("key1", ayy));
EXPECT_NE(ds, d1["key1"]);
EXPECT_EQ(ds, std::move(d2).getDefault("not-a-key", tmp));
EXPECT_EQ(dynamic(dynamic::object("key2", s)), d2);
EXPECT_EQ(ds, tmp);
// rvalue - rvalue
EXPECT_EQ(ds, std::move(d3).getDefault("key3", std::move(tmp)));
EXPECT_NE(ds, d3["key3"]);
EXPECT_EQ(ds, tmp);
EXPECT_EQ(ds, std::move(d4).getDefault("not-a-key", std::move(tmp)));
EXPECT_EQ(dynamic(dynamic::object("key4", s)), d4);
EXPECT_NE(ds, tmp);
}
TEST(Dynamic, GetString) {
const dynamic c(make_long_string());
dynamic d(make_long_string());
dynamic m(make_long_string());
auto s = make_long_string();
EXPECT_EQ(s, c.getString());
EXPECT_EQ(s, c.getString());
d.getString() += " hello";
EXPECT_EQ(s + " hello", d.getString());
EXPECT_EQ(s + " hello", d.getString());
EXPECT_EQ(s, std::move(m).getString());
EXPECT_NE(dynamic(s), m);
}
TEST(Dynamic, GetSmallThings) {
const dynamic cint(5);
const dynamic cdouble(5.0);
const dynamic cbool(true);
dynamic dint(5);
dynamic ddouble(5.0);
dynamic dbool(true);
dynamic mint(5);
dynamic mdouble(5.0);
dynamic mbool(true);
EXPECT_EQ(5, cint.getInt());
dint.getInt() = 6;
EXPECT_EQ(6, dint.getInt());
EXPECT_EQ(5, std::move(mint).getInt());
EXPECT_EQ(5.0, cdouble.getDouble());
ddouble.getDouble() = 6.0;
EXPECT_EQ(6.0, ddouble.getDouble());
EXPECT_EQ(5.0, std::move(mdouble).getDouble());
EXPECT_EQ(true, cbool.getBool());
dbool.getBool() = false;
EXPECT_FALSE(dbool.getBool());
EXPECT_EQ(true, std::move(mbool).getBool());
}
TEST(Dynamic, At) {
const dynamic cd = dynamic::object("key1", make_long_string());
dynamic dd = dynamic::object("key1", make_long_string());
dynamic md = dynamic::object("key1", make_long_string());
dynamic ds(make_long_string());
EXPECT_EQ(ds, cd.at("key1"));
EXPECT_EQ(ds, cd.at("key1"));
dd.at("key1").getString() += " hello";
EXPECT_EQ(dynamic(make_long_string() + " hello"), dd.at("key1"));
EXPECT_EQ(dynamic(make_long_string() + " hello"), dd.at("key1"));
EXPECT_EQ(ds, std::move(md).at("key1"));
EXPECT_NE(ds, md.at("key1"));
}
TEST(Dynamic, Brackets) {
const dynamic cd = dynamic::object("key1", make_long_string());
dynamic dd = dynamic::object("key1", make_long_string());
dynamic md = dynamic::object("key1", make_long_string());
dynamic ds(make_long_string());
EXPECT_EQ(ds, cd["key1"]);
EXPECT_EQ(ds, cd["key1"]);
dd["key1"].getString() += " hello";
EXPECT_EQ(dynamic(make_long_string() + " hello"), dd["key1"]);
EXPECT_EQ(dynamic(make_long_string() + " hello"), dd["key1"]);
EXPECT_EQ(ds, std::move(md)["key1"]);
EXPECT_NE(ds, md["key1"]);
}
int main(int argc, char** argv) { int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv); testing::InitGoogleTest(&argc, argv);
gflags::ParseCommandLineFlags(&argc, &argv, true); gflags::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