Skip to content

Commit 3464f5f

Browse files
committed
Squashed commit of the following:
commit 8b16df3 Author: Kirill Müller <krlmlr@users.noreply.github.com> Date: Thu Jan 22 07:37:45 2026 +0100 Formatting commit f557b0b Author: Kirill Müller <krlmlr@users.noreply.github.com> Date: Thu Jan 22 07:35:10 2026 +0100 Clarify commit ac9756b Author: Kirill Müller <krlmlr@users.noreply.github.com> Date: Thu Jan 22 07:06:41 2026 +0100 Remove commit 64be6e7 Author: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu Jan 22 07:05:34 2026 +0100 Add test for external_pointer attribute preservation and fix move assignment operator (r-lib#486) Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> commit 1db6beb Author: Kirill Müller <kirill@cynkra.com> Date: Sat Dec 7 20:47:09 2024 +0100 fix: Avoid premature release for external pointers
1 parent c031145 commit 3464f5f

File tree

2 files changed

+61
-2
lines changed

2 files changed

+61
-2
lines changed

cpp11test/src/test-external_pointer.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,53 @@ context("external_pointer-C++") {
3636
uniq.reset();
3737
expect_true(deleted == true);
3838
}
39+
40+
test_that("external_pointer preserves attributes when moved (issue #308)") {
41+
// Test move constructor
42+
{
43+
int* value = new int(42);
44+
cpp11::external_pointer<int> p(value);
45+
46+
// Set an attribute on the external pointer
47+
Rf_setAttrib(p, R_ClassSymbol, Rf_mkString("test_class"));
48+
49+
// Verify attribute exists before move
50+
SEXP class_attr = Rf_getAttrib(p, R_ClassSymbol);
51+
expect_true(class_attr != R_NilValue);
52+
53+
// Move the external pointer using move constructor
54+
cpp11::external_pointer<int> p_moved = std::move(p);
55+
56+
// Verify attribute is preserved after move
57+
SEXP class_attr_after = Rf_getAttrib(p_moved, R_ClassSymbol);
58+
expect_true(class_attr_after != R_NilValue);
59+
expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0);
60+
61+
// Clean up
62+
delete p_moved.release();
63+
}
64+
65+
// Test move assignment operator
66+
{
67+
int* value1 = new int(1);
68+
cpp11::external_pointer<int> p1(value1);
69+
70+
// Set an attribute on p1
71+
Rf_setAttrib(p1, R_ClassSymbol, Rf_mkString("test_class"));
72+
73+
// Create p2 with nullptr (no memory leak)
74+
cpp11::external_pointer<int> p2(nullptr);
75+
76+
// Move assign p1 to p2
77+
p2 = std::move(p1);
78+
79+
// Verify attribute is preserved after move assignment
80+
SEXP class_attr_after = Rf_getAttrib(p2, R_ClassSymbol);
81+
expect_true(class_attr_after != R_NilValue);
82+
expect_true(strcmp(CHAR(STRING_ELT(class_attr_after, 0)), "test_class") == 0);
83+
84+
// Clean up
85+
delete p2.release();
86+
}
87+
}
3988
}

inst/include/cpp11/external_pointer.hpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,19 @@ class external_pointer {
6767
data_ = safe[Rf_shallow_duplicate](rhs.data_);
6868
}
6969

70-
external_pointer(external_pointer&& rhs) { reset(rhs.release()); }
70+
external_pointer(external_pointer&& rhs) {
71+
data_ = rhs.data_;
72+
rhs.data_ = R_NilValue;
73+
}
7174

72-
external_pointer& operator=(external_pointer&& rhs) noexcept { reset(rhs.release()); }
75+
external_pointer& operator=(external_pointer&& rhs) noexcept {
76+
// This works even if `this == &rhs` because `data_` (a `cpp11::sexp`) handles
77+
// the underlying resource.
78+
data_ = rhs.data_;
79+
// Order matters: first assign, then clear the RHS.
80+
rhs.data_ = R_NilValue;
81+
return *this;
82+
}
7383

7484
external_pointer& operator=(std::nullptr_t) noexcept { reset(); };
7585

0 commit comments

Comments
 (0)