Skip to content

Commit badd2f0

Browse files
authored
Merge pull request #28 from pachadotdev/issue452
Issue452
2 parents b03d10f + 224b9af commit badd2f0

File tree

6 files changed

+186
-17
lines changed

6 files changed

+186
-17
lines changed

R/source.R

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
#' uses 'CXX11' if unset.
1919
#' @param dir The directory to store the generated source files. `tempfile()` is
2020
#' used by default. The directory will be removed if `clean` is `TRUE`.
21+
#' @param local Passed to [dyn.load()]. If `TRUE` (the default) the shared
22+
#' library is loaded with local symbols; if `FALSE` symbols are made global
23+
#' (equivalent to `dyn.load(..., local = FALSE)`), which can be required when
24+
#' other shared objects need to see RTTI/vtable symbols from this library.
25+
#' @note See the unit test that demonstrates this usage at
26+
#' \code{tests/testthat/test-source-local.R} (shows how `local = FALSE` exports
27+
#' the necessary symbols so separate shared objects can link against them).
2128
#' @return For [cpp_source()] and `[cpp_function()]` the results of
2229
#' [dyn.load()] (invisibly). For `[cpp_eval()]` the results of the evaluated
2330
#' expression.
@@ -65,7 +72,7 @@
6572
#' }
6673
#'
6774
#' @export
68-
cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11"), dir = tempfile()) {
75+
cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11"), dir = tempfile(), local = TRUE) {
6976
stop_unless_installed(c("brio", "callr", "cli", "decor", "desc", "glue", "tibble", "vctrs"))
7077
if (!missing(file) && !file.exists(file)) {
7178
stop("Can't find `file` at this path:\n", file, "\n", call. = FALSE)
@@ -145,7 +152,7 @@ cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, qu
145152
brio::write_lines(r_functions, r_path)
146153
source(r_path, local = env)
147154

148-
dyn.load(shared_lib, local = TRUE, now = TRUE)
155+
dyn.load(shared_lib, local = local, now = TRUE)
149156
}
150157

151158
the <- new.env(parent = emptyenv())
@@ -183,7 +190,7 @@ generate_makevars <- function(includes, cxx_std) {
183190

184191
#' @rdname cpp_source
185192
#' @export
186-
cpp_function <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11")) {
193+
cpp_function <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11"), local = TRUE) {
187194
cpp_source(code = paste(c('#include "cpp11.hpp"',
188195
"using namespace ::cpp11;",
189196
"namespace writable = ::cpp11::writable;",
@@ -193,15 +200,16 @@ cpp_function <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE,
193200
env = env,
194201
clean = clean,
195202
quiet = quiet,
196-
cxx_std = cxx_std
203+
cxx_std = cxx_std,
204+
local = local
197205
)
198206
}
199207

200208
utils::globalVariables("f")
201209

202210
#' @rdname cpp_source
203211
#' @export
204-
cpp_eval <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11")) {
212+
cpp_eval <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11"), local = TRUE) {
205213
cpp_source(code = paste(c('#include "cpp11.hpp"',
206214
"using namespace ::cpp11;",
207215
"namespace writable = ::cpp11::writable;",
@@ -214,7 +222,8 @@ cpp_eval <- function(code, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx
214222
env = env,
215223
clean = clean,
216224
quiet = quiet,
217-
cxx_std = cxx_std
225+
cxx_std = cxx_std,
226+
local = local
218227
)
219228
f()
220229
}

inst/include/cpp11/as.hpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -296,18 +296,14 @@ template <typename Container, typename AsCstring>
296296
SEXP as_sexp_strings(const Container& from, AsCstring&& c_str) {
297297
R_xlen_t size = from.size();
298298

299-
SEXP data;
300-
try {
301-
data = PROTECT(safe[Rf_allocVector](STRSXP, size));
299+
SEXP data = PROTECT(safe[Rf_allocVector](STRSXP, size));
302300

301+
unwind_protect([&] {
303302
auto it = from.begin();
304303
for (R_xlen_t i = 0; i < size; ++i, ++it) {
305-
SET_STRING_ELT(data, i, safe[Rf_mkCharCE](c_str(*it), CE_UTF8));
304+
SET_STRING_ELT(data, i, Rf_mkCharCE(c_str(*it), CE_UTF8));
306305
}
307-
} catch (const unwind_exception& e) {
308-
UNPROTECT(1);
309-
throw e;
310-
}
306+
});
311307

312308
UNPROTECT(1);
313309
return data;

inst/include/cpp11/integers.hpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,26 @@ inline integers as_integers(SEXP x) {
102102
}
103103

104104
} // namespace cpp11
105+
106+
// Note: Proxy Behavior in writable::integers
107+
//
108+
// When using writable::integers, operator[] returns a proxy object that allows
109+
// both reading and writing. For cases where you need the actual int value
110+
// (e.g., when using with C-style variadic functions like Rprintf), use one of
111+
// these three approaches:
112+
//
113+
// 1. Direct value access: vec.value(i) [Recommended]
114+
// 2. Explicit cast: (int)vec[i]
115+
// 3. Auto with explicit type: int val = vec[i];
116+
//
117+
// Example demonstrating the issue and solutions:
118+
// writable::integers vec;
119+
// vec.push_back(42);
120+
//
121+
// // This may print garbage due to proxy object:
122+
// // Rprintf("Value: %d\n", vec[0]); // DON'T DO THIS
123+
//
124+
// // These all work correctly:
125+
// Rprintf("Value: %d\n", vec.value(0)); // Recommended
126+
// Rprintf("Value: %d\n", (int)vec[0]); // Also works
127+
// int val = vec[0]; Rprintf("Value: %d\n", val); // Also works

inst/include/cpp11/r_vector.hpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,15 @@ class r_vector : public cpp11::r_vector<T> {
256256

257257
iterator find(const r_string& name) const;
258258

259+
/// Get the value at position without returning a proxy
260+
/// This is useful when you need the actual value (e.g., for C-style printf functions)
261+
/// that don't trigger implicit conversions from proxy types
262+
#ifdef LONG_VECTOR_SUPPORT
263+
T value(const int pos) const;
264+
#endif
265+
T value(const R_xlen_t pos) const;
266+
T value(const size_type pos) const;
267+
259268
attribute_proxy<r_vector<T>> attr(const char* name) const;
260269
attribute_proxy<r_vector<T>> attr(const std::string& name) const;
261270
attribute_proxy<r_vector<T>> attr(SEXP name) const;
@@ -1156,6 +1165,24 @@ inline typename r_vector<T>::iterator r_vector<T>::find(const r_string& name) co
11561165
return end();
11571166
}
11581167

1168+
#ifdef LONG_VECTOR_SUPPORT
1169+
template <typename T>
1170+
inline T r_vector<T>::value(const int pos) const {
1171+
return value(static_cast<R_xlen_t>(pos));
1172+
}
1173+
#endif
1174+
1175+
template <typename T>
1176+
inline T r_vector<T>::value(const R_xlen_t pos) const {
1177+
// Use the parent read-only class's operator[] which returns T directly
1178+
return cpp11::r_vector<T>::operator[](pos);
1179+
}
1180+
1181+
template <typename T>
1182+
inline T r_vector<T>::value(const size_type pos) const {
1183+
return value(static_cast<R_xlen_t>(pos));
1184+
}
1185+
11591186
template <typename T>
11601187
inline attribute_proxy<r_vector<T>> r_vector<T>::attr(const char* name) const {
11611188
return attribute_proxy<r_vector<T>>(*this, name);

man/cpp_source.Rd

Lines changed: 16 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/test-source-local.R

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
test_that("cpp_source local controls RTTI/vtable symbol visibility", {
2+
skip_on_os("windows")
3+
4+
mk_dirs <- function() {
5+
d1 <- tempfile("cpp_source_local1")
6+
d2 <- tempfile("cpp_source_local2")
7+
dir.create(d1); dir.create(d2)
8+
list(provider = d1, consumer = d2)
9+
}
10+
11+
unload_dirs <- function(dirs) {
12+
dlls <- getLoadedDLLs()
13+
for (nm in names(dlls)) {
14+
# Some R builds/platforms expose different DLL info; access $path
15+
# defensively to avoid errors when the structure differs (macOS).
16+
p <- tryCatch({ dlls[[nm]]$path }, error = function(e) NULL)
17+
if (!is.null(p)) {
18+
for (d in dirs) {
19+
if (grepl(d, p, fixed = TRUE)) {
20+
tryCatch(dyn.unload(p), error = function(e) NULL)
21+
}
22+
}
23+
}
24+
}
25+
}
26+
27+
provider_so_path <- function(dir) {
28+
src <- file.path(dir, "src")
29+
files <- list.files(src, pattern = paste0("\\\\", .Platform$dynlib.ext, "$"), ignore.case = TRUE)
30+
if (length(files) == 0) return(character())
31+
file.path(src, files[[1]])
32+
}
33+
34+
dirs <- mk_dirs()
35+
on.exit({
36+
unload_dirs(unlist(dirs))
37+
unlink(unlist(dirs), recursive = TRUE, force = TRUE)
38+
}, add = TRUE)
39+
40+
# Provider: abstract Base + Impl factory (polymorphic triggers RTTI/vtable)
41+
provider_code <- '
42+
#include <cpp11/R.hpp>
43+
struct Base { virtual ~Base(){}; virtual int foo() = 0; };
44+
struct Impl : Base { int foo() override { return 77; } };
45+
46+
extern "C" Base* make_impl() { return new Impl(); }
47+
extern "C" void destroy_impl(Base* p) { delete p; }
48+
'
49+
50+
# Consumer uses typeid(Base) (forces reference to typeinfo symbol) and
51+
# calls the factory produced by the provider.
52+
consumer_code <- '\n#include <cpp11/R.hpp>\n#include <typeinfo>\n#include <string>\nstruct Base { virtual ~Base(){}; virtual int foo() = 0; };\nextern "C" Base* make_impl();\nextern "C" SEXP call_typeinfo_and_run() {\n const std::type_info& t = typeid(Base);\n std::string n = t.name();\n Base* b = make_impl();\n int v = b->foo();\n delete b;\n SEXP out = PROTECT(Rf_allocVector(INTSXP, 2));\n INTEGER(out)[0] = (int)n.size();\n INTEGER(out)[1] = v;\n UNPROTECT(1);\n return out;\n}\n'
53+
54+
# 1) provider loaded with local = TRUE -> consumer should fail to load
55+
expect_silent(cpp_source(code = provider_code, dir = dirs$provider, clean = FALSE, local = TRUE))
56+
expect_error(
57+
cpp_source(code = consumer_code, dir = dirs$consumer, clean = FALSE),
58+
regexp = "undefined symbol|symbol .* not found|undefined reference|symbol not found in flat namespace",
59+
ignore.case = TRUE
60+
)
61+
62+
# Clean up partial loads
63+
unload_dirs(unlist(dirs))
64+
65+
# 2) provider loaded with local = FALSE -> consumer loads and runs
66+
expect_silent(cpp_source(code = provider_code, dir = dirs$provider, clean = FALSE, local = FALSE))
67+
expect_silent(cpp_source(code = consumer_code, dir = dirs$consumer, clean = FALSE))
68+
69+
res <- .Call("call_typeinfo_and_run")
70+
expect_true(is.integer(res) && length(res) == 2)
71+
expect_equal(as.integer(res)[2], 77L)
72+
expect_true(as.integer(res)[1] > 0)
73+
74+
# Explicit check that the manual dyn.load(...) workaround is unnecessary.
75+
# Emulate the snippet to locate the provider shared object and show that it
76+
# exists; we already demonstrated the consumer works without running this
77+
# manual snippet because cpp_source(local = FALSE) provided global symbols.
78+
# Try to determine the built provider shared object path. On some runners
79+
# the file may not be discoverable via listing (packaged R builds, macOS
80+
# variations). Fall back to inspecting getLoadedDLLs(); if still not
81+
# available, skip the explicit dyn.load check on this platform.
82+
so_path <- provider_so_path(dirs$provider)
83+
if (length(so_path) == 0) {
84+
dlls <- getLoadedDLLs()
85+
for (nm in names(dlls)) {
86+
p <- tryCatch({ dlls[[nm]]$path }, error = function(e) NULL)
87+
if (!is.null(p) && grepl(dirs$provider, p, fixed = TRUE)) {
88+
so_path <- p
89+
break
90+
}
91+
}
92+
}
93+
94+
if (length(so_path) == 0 || !nzchar(so_path)) {
95+
skip("Could not locate provider shared object on this platform; skipping manual dyn.load check")
96+
}
97+
98+
expect_true(file.exists(so_path))
99+
# Loading it manually with local = FALSE would succeed, but wasn't required.
100+
expect_silent(dyn.load(so_path, local = FALSE, now = TRUE))
101+
})

0 commit comments

Comments
 (0)