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

diff --git a/folly/dynamic-inl.h b/folly/dynamic-inl.h
index 6db141bec9784fd49e3dfc5037c043f23ffa5603..6b42fab2b93d99e7c54162b451e122be84659050 100644 (file)
--- a/folly/dynamic-inl.h
+++ b/folly/dynamic-inl.h
@@ -394,8 +394,8 @@ 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::c_str() const { return get<fbstring>().c_str(); }
+inline const char* dynamic::data()  const& { return get<fbstring>().data();  }
+inline const char* dynamic::c_str() const& { return get<fbstring>().c_str(); }
 inline StringPiece dynamic::stringPiece() const { return get<fbstring>(); }
 
 template<class T>
@@ -470,7 +470,7 @@ template<class K, class V> inline dynamic& dynamic::setDefault(K&& k, V&& v) {
                                    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));
 }
 
@@ -597,7 +597,7 @@ T dynamic::asImpl() const {
 
 // Return a T* to our type, or null if we're not that type.
 template<class T>
-T* dynamic::get_nothrow() noexcept {
+T* dynamic::get_nothrow() & noexcept {
   if (type_ != TypeInfo<T>::type) {
     return nullptr;
   }
@@ -605,7 +605,7 @@ T* dynamic::get_nothrow() noexcept {
 }
 
 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>();
 }
 

diff --git a/folly/dynamic.cpp b/folly/dynamic.cpp
index e58292447f27b0bcc916b404b57a1de73c75200b..f7be57e8ca0dc5e9cb5fc00a014bee0aecb74313 100644 (file)
--- a/folly/dynamic.cpp
+++ b/folly/dynamic.cpp
@@ -160,7 +160,7 @@ dynamic&& dynamic::getDefault(const dynamic& k, dynamic&& v) const {
   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 (!idx.isInt()) {
       throw TypeError("int64", idx.type());

diff --git a/folly/dynamic.h b/folly/dynamic.h
index b0444d9be688cb2a13894ad3f14d21cbb1c3b97b..5bd36f4c54aeedf7bc7d868cfd8a09fa6fdefe34 100644 (file)
--- a/folly/dynamic.h
+++ b/folly/dynamic.h
@@ -298,8 +298,10 @@ public:
    *
    * These will throw a TypeError if the dynamic is not a string.
    */
-  const char* data()  const;
-  const char* c_str() const;
+  const char* data()  const&;
+  const char* data()  && = delete;
+  const char* c_str() const&;
+  const char* c_str() && = delete;
   StringPiece stringPiece() const;
 
   /*
@@ -374,8 +376,9 @@ public:
    * Using these with dynamic objects that are not arrays or objects
    * will throw a TypeError.
    */
-  const dynamic* get_ptr(dynamic const&) const;
-  dynamic* get_ptr(dynamic const&);
+  const dynamic* get_ptr(dynamic const&) const&;
+  dynamic* get_ptr(dynamic const&) &;
+  dynamic* get_ptr(dynamic const&) && = delete;
 
   /*
    * This works for access to both objects and arrays.
@@ -494,8 +497,9 @@ private:
 
   template<class T> T const& get() const;
   template<class T> T&       get();
-  template<class T> T*       get_nothrow() noexcept;
-  template<class T> T const* get_nothrow() const noexcept;
+  template<class T> T*       get_nothrow() & 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 const* getAddress() const noexcept;