Stop double protecting writable vectors (#365)

* Stop double protecting writable vectors

And fix memory leak when assigning a `writable::r_vector<T>&&` temporary value to an existing `writable::r_vector<T>`.

* NEWS bullet

* Add read only move constructor and move assignment operator

* Add more assignment operator tests

* Add some extra comments on read only vector from writable vector ctor

* Slightly more explicit writable from read only lvalue ctor

* Fix issue related to writable move constructor

And add more related tests

* Add extra test for capacity tracking

* Add a test for the read only copy constructor from writable vectors

* Tiny tweak

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,10 @@
 # cpp11 (development version)
 
+* Read only `r_vector`s now have a move constructor and move assignment
+  operator (#365).
+
+* Fixed an issue where writable vectors were being protected twice (#365).
+
 * Removed usage of the following non-API functions:
   * `SETLENGTH()`
   * `SET_TRUELENGTH()`

cpp11test/src/test-doubles.cpp

@@ -233,7 +233,11 @@ context("doubles-C++") {
     w = z;
     expect_true(w.size() == 5);
     expect_true(w.data() != z.data());
-    expect_true(w.is_altrep() == z.is_altrep());
+    // Shallow duplication of objects of a very small size (like 1:5) don't result in
+    // a new ALTREP object. Make sure we check ALTREP-ness of the newly duplicated object,
+    // instead of just blindly inheriting the ALTREP-ness of the thing we duplicate.
+    expect_true(w.is_altrep() != z.is_altrep());
+    expect_true(w.is_altrep() == ALTREP(w.data()));
   }
 
   test_that("writable::doubles(SEXP) move assignment") {

cpp11test/src/test-integers.cpp

@@ -2,7 +2,6 @@
 #include "cpp11/doubles.hpp"
 #include "cpp11/function.hpp"
 #include "cpp11/integers.hpp"
-#include "cpp11/protect.hpp"
 #include "cpp11/strings.hpp"
 
 #include <testthat.h>
@@ -208,22 +207,4 @@ context("integers-C++") {
     int y = NA_INTEGER;
     expect_true(cpp11::is_na(y));
   }
-
-  test_that("writable integer vector temporary isn't leaked (#338)") {
-    R_xlen_t before = cpp11::detail::store::count();
-
-    // +1 from `x` allocation
-    cpp11::writable::integers x(1);
-
-    // Calls move assignment operator `operator=(r_vector<T>&& rhs)`
-    // +1 from `rhs` allocation and move into `x`
-    // -1 from old `x` release
-    x = cpp11::writable::integers(1);
-
-    R_xlen_t after = cpp11::detail::store::count();
-
-    expect_true(before == 0);
-    // TODO: This should be 1 but writable vectors are being double protected
-    expect_true(after - before == 2);
-  }
 }

cpp11test/src/test-list.cpp

@@ -163,22 +163,4 @@ context("list-C++") {
     expect_true(Rf_xlength(y) == 0);
     expect_true(y != R_NilValue);
   }
-
-  test_that("writable list vector temporary isn't leaked (#338)") {
-    R_xlen_t before = cpp11::detail::store::count();
-
-    // +1 from `x` allocation
-    cpp11::writable::list x(1);
-
-    // Calls move assignment operator `operator=(r_vector<T>&& rhs)`
-    // +1 from `rhs` allocation and move into `x`
-    // -1 from old `x` release
-    x = cpp11::writable::list(1);
-
-    R_xlen_t after = cpp11::detail::store::count();
-
-    expect_true(before == 0);
-    // TODO: This should be 1 but writable vectors are being double protected
-    expect_true(after - before == 2);
-  }
 }