Merge pull request #246 from RcppCore/feature/xptr-improvements

Improvements to XPtr

Author: jjallaire

ChangeLog

@@ -1,3 +1,12 @@
+2014-02-03  JJ Allaire  <jj@rstudio.org>
+
+        * inst/include/Rcpp/XPtr.h: Improvements to XPtr including new
+        checked_get and release functions and improved behavior (throw
+        an exception rather than crash) when a NULL external pointer is
+        dereferenced.
+        * inst/unitTests/runit.XPTr.R: tests for XPtr improvements.
+        * inst/unitTests/cpp/XPtr.cpp: tests for XPtr improvements.
+
 2014-02-03  JJ Allaire  <jj@rstudio.org>
 
         * R/Attributes.R: Include pkg_types.h file in RcppExports.cpp

inst/NEWS.Rd

@@ -8,6 +8,10 @@
     \itemize{
       \item Defining an error handler for tinyformat prevents \code{assert()}
       from spilling.
+      \item Improvements to XPtr including new \code{checked_get} and
+      \code{release} functions as well as improved behavior (throw an
+      exception rather than crash) when a NULL external pointer is
+      dereferenced.
       \item Evaluate R code within an \code{R_toplevelExec} block to prevent
       user interrupts from bypassing C++ destructors on the stack.
     }

inst/include/Rcpp/XPtr.h

@@ -98,28 +98,84 @@ class XPtr :
     	    return *this ;
     }
 
+    /**
+     * Typesafe accessor for underlying pointer (use checked_get
+     * if you want an exception thrown if the pointer is NULL)
+     */
+    inline T* get() const {
+        return (T*)(R_ExternalPtrAddr( Storage::get__() ));
+    }
+
+    /**
+     * Boolean operator wrapper for get() using the "safe bool idiom", see:
+     * http://www.boost.org/doc/libs/1_57_0/boost/smart_ptr/detail/operator_bool.hpp
+     */
+    typedef void (*unspecified_bool_type)();
+    static void unspecified_bool_true() {}
+    operator unspecified_bool_type() const
+    {
+        return get() == NULL ? 0 : unspecified_bool_true;
+    }
+    bool operator!() const
+    {
+        return get() == NULL;
+    }
+
+    /**
+     * Access underlying pointer throwing an exception if the ptr is NULL
+     */
+    inline T* checked_get() const {
+        T* ptr = get();
+        if (ptr == NULL)
+            throw ::Rcpp::exception("external pointer is not valid" ) ;
+        return ptr;
+    }
+
     /**
      * Returns a reference to the object wrapped. This allows this
      * object to look and feel like a dumb pointer to T
      */
     T& operator*() const {
-        return *((T*)R_ExternalPtrAddr( Storage::get__() )) ;
+        return *(checked_get()) ;
     }
 
     /**
      * Returns the dumb pointer. This allows to call the -> operator
      * on this as if it was the dumb pointer
      */
     T* operator->() const {
-         return (T*)(R_ExternalPtrAddr( Storage::get__() ));
+        return checked_get() ;
     }
 
     void setDeleteFinalizer() {
         R_RegisterCFinalizerEx( Storage::get__(), finalizer_wrapper<T,Finalizer> , FALSE) ;
     }
 
+    /**
+     * Release the external pointer (if any) immediately. This will cause
+     * the pointer to be deleted and it's storage to be set to NULL.
+     * After this call the get() method returns NULL and the checked_get()
+     * method throws an exception.
+     *
+     * See the discussion here for the basic logic behind release:
+     * https://stat.ethz.ch/pipermail/r-help/2007-August/137871.html
+     */
+    void release() {
+
+        if (get() != NULL)
+        {
+            // Call the finalizer -- note that this implies that finalizers
+            // need to be ready for a NULL external pointer value (our
+            // default C++ finalizer is since delete NULL is a no-op).
+            finalizer_wrapper<T,Finalizer>( Storage::get__() );
+
+            // Clear the external pointer
+            R_ClearExternalPtr( Storage::get__() );
+        }
+    }
+
     inline operator T*(){
-        return (T*)( R_ExternalPtrAddr( Storage::get__() )) ;
+        return checked_get() ;
     }
 
     void update(SEXP){}

inst/unitTests/cpp/XPtr.cpp

@@ -46,3 +46,24 @@ int xptr_2( XPtr< std::vector<int> > p){
     		return p->front() ;
 }
 
+// [[Rcpp::export]]
+bool xptr_release( XPtr< std::vector<int> > p) {
+    p.release();
+    return !p;
+}
+
+// [[Rcpp::export]]
+bool xptr_access_released( XPtr< std::vector<int> > p) {
+
+    // double-release should be a no-op
+    p.release();
+
+    // get should return NULL
+    return p.get() == NULL;
+}
+
+// [[Rcpp::export]]
+int xptr_use_released( XPtr< std::vector<int> > p ) {
+    return p->front();
+}
+

inst/unitTests/runit.XPTr.R

@@ -30,6 +30,14 @@ if (.runThisTest) {
 
         front <- xptr_2(xp)
         checkEquals( front, 1L, msg = "check usage of external pointer" )
+
+        checkTrue(xptr_release(xp), msg = "check release of external pointer")
+
+        checkTrue(xptr_access_released(xp), msg = "check access of released external pointer")
+
+        checkException(xptr_use_released(xp),
+                       msg = "check exception on use of released external pointer",
+                       silent = TRUE)
     }
 
 }