diff --git a/cpp11test/src/test-external_pointer.cpp b/cpp11test/src/test-external_pointer.cpp index 897fd4f2..d8b960aa 100644 --- a/cpp11test/src/test-external_pointer.cpp +++ b/cpp11test/src/test-external_pointer.cpp @@ -36,4 +36,53 @@ context("external_pointer-C++") { uniq.reset(); expect_true(deleted == true); } + + test_that("external_pointer preserves attributes when moved (issue #308)") { + // Test move constructor + { + int* value = new int(42); + cpp11::external_pointer p(value); + + // Set an attribute on the external pointer + Rf_setAttrib(p, R_ClassSymbol, Rf_mkString("test_class")); + + // Verify attribute exists before move + SEXP class_attr = Rf_getAttrib(p, R_ClassSymbol); + expect_true(class_attr != R_NilValue); + + // Move the external pointer using move constructor + cpp11::external_pointer p_moved = std::move(p); + + // Verify attribute is preserved after move + SEXP class_attr_after = Rf_getAttrib(p_moved, R_ClassSymbol); + expect_true(class_attr_after != R_NilValue); + expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0); + + // Clean up + delete p_moved.release(); + } + + // Test move assignment operator + { + int* value1 = new int(1); + cpp11::external_pointer p1(value1); + + // Set an attribute on p1 + Rf_setAttrib(p1, R_ClassSymbol, Rf_mkString("test_class")); + + // Create p2 with nullptr (no memory leak) + cpp11::external_pointer p2(nullptr); + + // Move assign p1 to p2 + p2 = std::move(p1); + + // Verify attribute is preserved after move assignment + SEXP class_attr_after = Rf_getAttrib(p2, R_ClassSymbol); + expect_true(class_attr_after != R_NilValue); + expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0); + + // Clean up + delete p2.release(); + } + } } diff --git a/inst/include/cpp11/external_pointer.hpp b/inst/include/cpp11/external_pointer.hpp index a62134ec..04d8fb4a 100644 --- a/inst/include/cpp11/external_pointer.hpp +++ b/inst/include/cpp11/external_pointer.hpp @@ -66,9 +66,19 @@ class external_pointer { data_ = safe[Rf_shallow_duplicate](rhs.data_); } - external_pointer(external_pointer&& rhs) { reset(rhs.release()); } + external_pointer(external_pointer&& rhs) { + data_ = rhs.data_; + rhs.data_ = R_NilValue; + } - external_pointer& operator=(external_pointer&& rhs) noexcept { reset(rhs.release()); } + external_pointer& operator=(external_pointer&& rhs) noexcept { + // This works even if `this == &rhs` because `data_` (a `cpp11::sexp`) handles + // the underlying resource. + data_ = rhs.data_; + // Order matters: first assign, then clear the RHS. + rhs.data_ = R_NilValue; + return *this; + } external_pointer& operator=(std::nullptr_t) noexcept { reset(); };