Skip to content

Commit 396f354

Browse files
authored
Merge branch 'master' into fix/macos_ci
2 parents f470b14 + 5308aa7 commit 396f354

10 files changed

Lines changed: 175 additions & 9 deletions

File tree

‎ChangeLog‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,51 @@
1+
12
2026-10-31 Dirk Eddelbuettel <edd@debian.org>
23

34
* .github/workflows/macos.yaml (jobs): Retire macos-14 (but retain
45
macos-latest)
56

7+
2026-10-29 Kevin Ushey <kevinushey@gmail.com>
8+
9+
* inst/include/Rcpp/module/Module_Add_Property.h: Return *this from
10+
the property() overload taking a free-function getter and a member
11+
function setter, which fell off the end and trapped at module load
12+
(#1519)
13+
* inst/tinytest/cpp/Module.cpp: Expose one property per property()
14+
overload
15+
* inst/tinytest/test_module.R: Test them
16+
17+
* src/module.cpp (InternalFunction_invoke): Raise an error when fewer
18+
arguments are supplied than the arity stored in the function pointer's
19+
tag, instead of reading the missing ones from uninitialized memory
20+
(#1521)
21+
* inst/include/Rcpp/module/Module.h (Module::get_function): Store the
22+
arity of module functions in the pointer tag
23+
* inst/include/Rcpp/InternalFunction.h: Idem for InternalFunction
24+
* inst/tinytest/cpp/Module.cpp: Add module functions with formals
25+
* inst/tinytest/test_module.R: Test calls with too few arguments
26+
* inst/tinytest/test_internal_function.R: Idem
27+
* inst/tinytest/test_internal_function_cpp11.R: Idem
28+
29+
* R/00_classes.R (.cppfunction_formals_gets): Find the .External() call
30+
in the body of the function instead of assuming it is the second
31+
element, which it is not once coverage tools have instrumented the body
32+
* R/00_classes.R (.cppfunction_set_external_args): New helper
33+
* inst/tinytest/test_module.R: Test it with an instrumented body
34+
635
2026-10-23 Dirk Eddelbuettel <edd@debian.org>
736

837
* DESCRIPTION (Version, Date): Roll micro version and date
938
* inst/include/Rcpp/config.h: Idem
1039

40+
2026-10-22 Kevin Ushey <kevinushey@gmail.com>
41+
42+
* src/module.cpp (CppObject__finalize): Make finalize() a no-op for a
43+
module object that was never constructed (created without arguments for a
44+
class without a default constructor); it used to fail on the dummy
45+
pointer, and the failure surfaced as an error message at garbage
46+
collection
47+
* inst/tinytest/test_module.R: Add test
48+
1149
2026-10-22 Michael Sumner <mdsumner@gmail.com>
1250

1351
* R/00_classes.R: Define C++Field, C++OverloadedMethods and

‎R/00_classes.R‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,23 @@ setClass( "C++Function",
117117
contains = "function"
118118
)
119119

120-
.cppfunction_formals_gets <- function (fun, envir = environment(fun), value) {
121-
bd <- body(fun)
122-
b2 <- bd[[2L]]
123-
for( i in seq_along(value) ){
124-
b2[[3L+i]] <- as.name( names(value)[i] )
120+
## passes the arguments named in 'names' on from the .External(entry,
121+
## pointer, ...) call in expr; the call is searched for rather than assumed
122+
## to be a top-level statement, as coverage tools wrap each statement
123+
.cppfunction_set_external_args <- function( expr, names ){
124+
if( !is.call( expr ) )
125+
return( expr )
126+
if( identical( expr[[1L]], as.name( ".External" ) ) )
127+
return( as.call( c( as.list( expr )[1:3], lapply( names, as.name ) ) ) )
128+
for( i in seq_along( expr )[-1L] ){
129+
if( is.call( expr[[i]] ) )
130+
expr[[i]] <- .cppfunction_set_external_args( expr[[i]], names )
125131
}
126-
bd[[2]] <- b2
132+
expr
133+
}
134+
135+
.cppfunction_formals_gets <- function (fun, envir = environment(fun), value) {
136+
bd <- .cppfunction_set_external_args( body(fun), names(value) )
127137
f <- fun@.Data
128138
formals(f) <- value
129139
body(f) <- bd

‎inst/include/Rcpp/InternalFunction.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,24 @@ namespace Rcpp{
3535

3636
RCPP_GENERATE_CTOR_ASSIGN(InternalFunction_Impl)
3737

38+
// the arity goes in the pointer tag, where InternalFunction_invoke
39+
// checks it against the number of supplied arguments
40+
3841
template <typename RESULT_TYPE, typename... Args>
3942
InternalFunction_Impl(const std::function<RESULT_TYPE(Args...)> &fun) {
43+
Shield<SEXP> arity(Rf_ScalarInteger(static_cast<int>(sizeof...(Args))));
4044
set(
4145
XPtr<Rcpp::InternalFunctionWithStdFunction::CppFunctionBaseFromStdFunction<RESULT_TYPE, Args...> >(
4246
new Rcpp::InternalFunctionWithStdFunction::CppFunctionBaseFromStdFunction<RESULT_TYPE, Args...>(fun),
43-
false
47+
false,
48+
arity
4449
)
4550
);
4651
}
4752
template <typename RESULT_TYPE, typename... T>
4853
InternalFunction_Impl(RESULT_TYPE (*fun)(T...)) {
49-
set(XPtr<CppFunctionN<RESULT_TYPE, T...> >(new CppFunctionN<RESULT_TYPE, T...>(fun), true));
54+
Shield<SEXP> arity(Rf_ScalarInteger(static_cast<int>(sizeof...(T))));
55+
set(XPtr<CppFunctionN<RESULT_TYPE, T...> >(new CppFunctionN<RESULT_TYPE, T...>(fun), true, arity));
5056
}
5157

5258
void update(SEXP){}

‎inst/include/Rcpp/module/Module.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,12 @@ namespace Rcpp {
164164
}
165165
std::string sign ;
166166
fun->signature( sign, name_.data() ) ;
167+
168+
// checked against the number of supplied arguments by InternalFunction_invoke
169+
Shield<SEXP> arity( Rf_ScalarInteger( fun->nargs() ) ) ;
170+
167171
return List::create(
168-
XPtr<CppFunction>( fun, false ),
172+
XPtr<CppFunction>( fun, false, arity ),
169173
fun->is_void(),
170174
fun->docstring,
171175
sign,

‎inst/include/Rcpp/module/Module_Add_Property.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
name_,
8484
new CppProperty_GetPointer_SetMethod<Class,PROP>(GetMethod, SetMethod, docstring)
8585
) ;
86+
return *this ;
8687
}
8788

8889
template <typename PROP>

‎inst/tinytest/cpp/Module.cpp‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,24 @@ class ModuleNum{
8989
int y;
9090
};
9191

92+
class ModuleProperties {
93+
public:
94+
ModuleProperties() : value(0.0) {}
95+
96+
double get() { return value; }
97+
double get_const() const { return value; }
98+
void set(double value_) { value = value_; }
99+
100+
double value;
101+
};
102+
103+
double ModuleProperties_get(ModuleProperties* p) {
104+
return p->value;
105+
}
106+
void ModuleProperties_set(ModuleProperties* p, double value) {
107+
p->value = value;
108+
}
109+
92110
class ModuleNumber{
93111
public:
94112
ModuleNumber() : x(0.0), y(0) {};
@@ -166,6 +184,10 @@ RCPP_MODULE(demoModule) {
166184
function("bla1" , &bla1 );
167185
function("bla2" , &bla2 );
168186

187+
// formals with defaults, and formals shorter than the parameter list
188+
function("foo_formals", &foo, List::create(_["x"] = 2, _["y"] = 10.0));
189+
function("foo_short_formals", &foo, List::create(_["x"] = 2));
190+
169191
function("test_reference", test_reference);
170192
function("test_const_reference", test_const_reference);
171193
function("test_const", test_const);
@@ -193,6 +215,21 @@ RCPP_MODULE(demoModule) {
193215
.property("y", &ModuleNum::getY)
194216
;
195217

218+
// one property per class_::property() overload: getters are member,
219+
// const member or free functions, setters are member or free functions
220+
class_<ModuleProperties>("ModuleProperties")
221+
.constructor()
222+
.property("get", &ModuleProperties::get)
223+
.property("get_const", &ModuleProperties::get_const)
224+
.property("get_free", &ModuleProperties_get, "free getter")
225+
.property("get_set", &ModuleProperties::get, &ModuleProperties::set)
226+
.property("get_const_set", &ModuleProperties::get_const, &ModuleProperties::set)
227+
.property("get_set_free", &ModuleProperties::get, &ModuleProperties_set)
228+
.property("get_const_set_free", &ModuleProperties::get_const, &ModuleProperties_set)
229+
.property("get_free_set", &ModuleProperties_get, &ModuleProperties::set)
230+
.property("get_free_set_free", &ModuleProperties_get, &ModuleProperties_set)
231+
;
232+
196233
class_<ModuleNumber>("ModuleNumber")
197234

198235
.constructor()

‎inst/tinytest/test_internal_function.R‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ Rcpp::sourceCpp("cpp/InternalFunction.cpp")
2424
fun <- getAdd()
2525
expect_equal( fun(10,32), 42 )
2626

27+
## too few arguments raise an error rather than reading the missing ones
28+
## from uninitialized memory (#1521)
29+
expect_error( fun(10), "expecting 2, got 1" )
30+
2731
# test.internal_function_concatenate <- function(){
2832
fun <- getConcatenate()
2933
expect_equal( fun("Hello"," World"), "Hello World" )

‎inst/tinytest/test_internal_function_cpp11.R‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,7 @@ expect_equal( fun(38), 42 )
2929
# test.internal_function_concatenate <- function(){
3030
fun <- getConcatenate()
3131
expect_equal( fun("Hello"," World"), "Hello World" )
32+
33+
## too few arguments raise an error rather than reading the missing ones
34+
## from uninitialized memory (#1521)
35+
expect_error( fun("Hello"), "expecting 2, got 1" )

‎inst/tinytest/test_module.R‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,21 @@ expect_true(is_void(bla@.Data))
4141
expect_true(is_void(bla1@.Data))
4242
expect_true(is_void(bla2@.Data))
4343

44+
## module functions must be called with at least as many arguments as
45+
## the C++ function takes, rather than reading the missing ones from
46+
## uninitialized memory (#1521)
47+
expect_error( foo( 2L ), "expecting 2, got 1" )
48+
expect_error( bar(), "expecting 1, got 0" )
49+
expect_equal( foo_formals(), 20.0 )
50+
expect_equal( foo_formals( y = 3.0 ), 6.0 )
51+
expect_error( foo_short_formals(), "expecting 2, got 1" )
52+
53+
## formals<- finds the .External() call wherever it is in the body, e.g.
54+
## inside the block that coverage tools (covr) wrap each statement in
55+
wrapped <- quote({ if (TRUE) { covr:::count("key"); .External(entry, pointer, ...) } })
56+
expect_equal( deparse( Rcpp:::.cppfunction_set_external_args( wrapped, c("x", "y") ) ),
57+
deparse( quote({ if (TRUE) { covr:::count("key"); .External(entry, pointer, x, y) } }) ) )
58+
4459
w <- new( ModuleWorld )
4560
expect_equal( w$greet(), "hello" )
4661
w$set( "hello world" )
@@ -85,6 +100,27 @@ expect_equal( w$x, 2.0 )
85100

86101
expect_error( { w$y <- 3 } )
87102

103+
## every class_::property() overload; the free getter + member setter one
104+
## did not return *this, which aborted loading the whole module (#1519)
105+
p <- new( ModuleProperties )
106+
p$get_set <- 1.0
107+
expect_equal( p$get_set, 1.0 )
108+
p$get_const_set <- 2.0
109+
expect_equal( p$get_const_set, 2.0 )
110+
p$get_set_free <- 3.0
111+
expect_equal( p$get_set_free, 3.0 )
112+
p$get_const_set_free <- 4.0
113+
expect_equal( p$get_const_set_free, 4.0 )
114+
p$get_free_set <- 5.0
115+
expect_equal( p$get_free_set, 5.0 )
116+
p$get_free_set_free <- 6.0
117+
expect_equal( p$get_free_set_free, 6.0 )
118+
119+
expect_equal( c(p$get, p$get_const, p$get_free), c(6.0, 6.0, 6.0) )
120+
expect_error( { p$get <- 7.0 } )
121+
expect_error( { p$get_const <- 7.0 } )
122+
expect_error( { p$get_free <- 7.0 } )
123+
88124
# test.Module.member <- function(){
89125
w <- new( ModuleNumber )
90126
expect_equal( w$x, 0.0 )
@@ -123,3 +159,10 @@ gctorture(TRUE)
123159
res <- gadget$value()
124160
gctorture(FALSE)
125161
expect_identical( res, c(1, 2, 3) )
162+
163+
# test.Module.finalize.dummy <- function(){
164+
## an object created without arguments for a class lacking a default
165+
## constructor holds the dummy pointer; finalizing it must be a no-op
166+
## rather than an error (which used to surface at garbage collection)
167+
r <- new( ModuleRandomizer )
168+
expect_identical( r$finalize(), NULL )

‎src/module.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,14 @@ RCPP_FUN_4(SEXP, CppField__set, XP_Class cl, SEXP field_xp, SEXP obj, SEXP value
108108
cl->setProperty(field_xp, obj, value);
109109
return R_NilValue;
110110
}
111+
112+
extern SEXP rcpp_dummy_pointer;
113+
111114
RCPP_FUN_2(SEXP, CppObject__finalize, XP_Class cl, SEXP obj) {
115+
// an object created without arguments for a class lacking a default
116+
// constructor holds the dummy pointer: there is nothing to finalize
117+
if (obj == rcpp_dummy_pointer)
118+
return R_NilValue;
112119
cl->run_finalizer(obj);
113120
return R_NilValue;
114121
}
@@ -119,6 +126,18 @@ BEGIN_RCPP
119126
SEXP p = CDR(args);
120127
XP_Function fun(CAR(p)); p = CDR(p);
121128
UNPACK_EXTERNAL_ARGS(cargs,p)
129+
130+
// Module::get_function() and InternalFunction store the arity in the
131+
// tag; without this check, parameters beyond those supplied would be
132+
// read from uninitialized slots of cargs
133+
SEXP arity = R_ExternalPtrTag(fun);
134+
if (TYPEOF(arity) == INTSXP && Rf_xlength(arity) == 1 && nargs < INTEGER(arity)[0])
135+
Rcpp::stop(
136+
"incorrect number of arguments: expecting %d, got %d",
137+
INTEGER(arity)[0],
138+
nargs
139+
);
140+
122141
return fun->operator()(cargs);
123142
END_RCPP
124143
}

0 commit comments

Comments
 (0)