Commit 8e48b79d authored by Nick Terrell's avatar Nick Terrell Committed by facebook-github-bot-4

Delete functions that return a pointer when the dynamic object is a rvalue.

Summary: This diff is not yet complete, I want to see the contbuild before I change the
functions that return references to member functions.

It is unsafe to return a pointer when the dynamic object is a rvalue, because if
the pointer escapes the expression after the object is destroyed, we go into
segfault / undefined behavior land.
I have deleted these overloads.  The amount of valid code that is now disallowed
is minimal.  The only valid case I can think of is returing a pointer and
passing it to a function in the same expression that does not save the pointer.
However, this case is also dangerous, because if the function you pass it to
decides to save the pointer for later, we are in trouble, e.g.

  save_ptr(dynamic("str").c_str())

Since there are simple workarounds (naming the object), I think that is a small
price to pay for the greatly increased safety.

The next step is to overload all members that return a reference to a member
to move the member out if the dynamic is a rvalue:

  const dynamic& at(dynamic const&) const&;
        dynamic& at(dynamic const&)      &;
        dynamic  at(dynamic const&)      &&;  // Move out

I also need to go over the code more carefully to make sure that nothing went
wrong.

Reviewed By: @marcinpe

Differential Revision: D2257914
parent 5532f19f
...@@ -394,8 +394,8 @@ inline double& dynamic::getDouble() { return get<double>(); } ...@@ -394,8 +394,8 @@ 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 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(); }
inline StringPiece dynamic::stringPiece() const { return get<fbstring>(); } inline StringPiece dynamic::stringPiece() const { return get<fbstring>(); }
template<class T> template<class T>
...@@ -470,7 +470,7 @@ template<class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) { ...@@ -470,7 +470,7 @@ template<class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) {
std::forward<V>(v))).first->second; std::forward<V>(v))).first->second;
} }
inline dynamic* dynamic::get_ptr(dynamic const& idx) { 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));
} }
...@@ -597,7 +597,7 @@ T dynamic::asImpl() const { ...@@ -597,7 +597,7 @@ T dynamic::asImpl() const {
// Return a T* to our type, or null if we're not that type. // Return a T* to our type, or null if we're not that type.
template<class T> template<class T>
T* dynamic::get_nothrow() noexcept { T* dynamic::get_nothrow() & noexcept {
if (type_ != TypeInfo<T>::type) { if (type_ != TypeInfo<T>::type) {
return nullptr; return nullptr;
} }
...@@ -605,7 +605,7 @@ T* dynamic::get_nothrow() noexcept { ...@@ -605,7 +605,7 @@ T* dynamic::get_nothrow() noexcept {
} }
template<class T> template<class T>
T const* dynamic::get_nothrow() const noexcept { T const* dynamic::get_nothrow() const& noexcept {
return const_cast<dynamic*>(this)->get_nothrow<T>(); return const_cast<dynamic*>(this)->get_nothrow<T>();
} }
......
...@@ -160,7 +160,7 @@ dynamic&& dynamic::getDefault(const dynamic& k, dynamic&& v) const { ...@@ -160,7 +160,7 @@ dynamic&& dynamic::getDefault(const dynamic& k, dynamic&& v) const {
return std::move(v); return std::move(v);
} }
const dynamic* dynamic::get_ptr(dynamic const& idx) const { const dynamic* dynamic::get_ptr(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());
......
...@@ -298,8 +298,10 @@ public: ...@@ -298,8 +298,10 @@ public:
* *
* These will throw a TypeError if the dynamic is not a string. * These will throw a TypeError if the dynamic is not a string.
*/ */
const char* data() const; const char* data() const&;
const char* c_str() const; const char* data() && = delete;
const char* c_str() const&;
const char* c_str() && = delete;
StringPiece stringPiece() const; StringPiece stringPiece() const;
/* /*
...@@ -374,8 +376,9 @@ public: ...@@ -374,8 +376,9 @@ public:
* Using these with dynamic objects that are not arrays or objects * Using these with dynamic objects that are not arrays or objects
* will throw a TypeError. * will throw a TypeError.
*/ */
const dynamic* get_ptr(dynamic const&) const; const dynamic* get_ptr(dynamic const&) const&;
dynamic* get_ptr(dynamic const&); dynamic* get_ptr(dynamic const&) &;
dynamic* get_ptr(dynamic const&) && = delete;
/* /*
* This works for access to both objects and arrays. * This works for access to both objects and arrays.
...@@ -494,8 +497,9 @@ private: ...@@ -494,8 +497,9 @@ private:
template<class T> T const& get() const; template<class T> T const& get() const;
template<class T> T& get(); template<class T> T& get();
template<class T> T* get_nothrow() noexcept; template<class T> T* get_nothrow() & noexcept;
template<class T> T const* get_nothrow() const noexcept; template<class T> T const* get_nothrow() const& noexcept;
template<class T> T* get_nothrow() && noexcept = delete;
template<class T> T* getAddress() noexcept; template<class T> T* getAddress() noexcept;
template<class T> T const* getAddress() const noexcept; template<class T> T const* getAddress() const noexcept;
......
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