diff --git a/ChangeLog b/ChangeLog index a4e52670c..7cff4e574 100644 --- a/ChangeLog +++ b/ChangeLog @@ -66,6 +66,13 @@ * inst/tinytest/cpp/Module.cpp (Module_scope_is_cleared): New * inst/tinytest/test_module.R: Test loading the failing module + * inst/include/Rcpp/module/class.h (class_::AddMethod): Count special + '[' methods once per name rather than once per overload, as complete() + expects; overloading '[[' dropped methods from completion or ran past + the end of the method map (#1529) + * inst/tinytest/cpp/Module.cpp: Add classes overloading '[[' + * inst/tinytest/test_module.R: Test their completions + * vignettes/rmd/Rcpp-modules.Rmd: Document that objects of exposed classes returned by value or reference are copied, while a returned pointer is owned and eventually deleted by R, so that returning this or diff --git a/inst/include/Rcpp/module/class.h b/inst/include/Rcpp/module/class.h index 9b8da703a..0959edc7f 100644 --- a/inst/include/Rcpp/module/class.h +++ b/inst/include/Rcpp/module/class.h @@ -254,9 +254,10 @@ typename map_vec_signed_method::iterator it = ptr->vec_methods.find( name_ ) ; if( it == ptr->vec_methods.end() ){ it = ptr->vec_methods.insert( vec_signed_method_pair( name_, new vec_signed_method() ) ).first ; + // complete() relies on this counting names, not overloads + if( *name_ == '[' ) ptr->specials++ ; } (it->second)->push_back( new signed_method_class(m, valid, docstring ) ) ; - if( *name_ == '[' ) ptr->specials++ ; return *this ; } diff --git a/inst/tinytest/cpp/Module.cpp b/inst/tinytest/cpp/Module.cpp index 603171c74..920a0c978 100644 --- a/inst/tinytest/cpp/Module.cpp +++ b/inst/tinytest/cpp/Module.cpp @@ -173,6 +173,27 @@ double ModuleBase_value(const ModuleBase& x) { return x.value; } +class ModuleBrackets { +public: + ModuleBrackets() : value(1.0) {} + + double at(int i) { return value * i; } + double at2(int i, int j) { return value * i * j; } + double total() { return value; } + + double value; +}; + +class ModuleBracketsOnly { +public: + ModuleBracketsOnly() : value(1.0) {} + + double at(int i) { return value * i; } + double at2(int i, int j) { return value * i * j; } + + double value; +}; + // counts its destructions, to check who owns objects returned from C++ class ModuleOwned { public: @@ -303,6 +324,22 @@ RCPP_MODULE(demoModule) { function("ModuleBase_value", &ModuleBase_value); + // overloaded [[, with and without an ordinary method + class_("ModuleBrackets") + .constructor() + .method("[[", &ModuleBrackets::at) + .method("[[", &ModuleBrackets::at2) + .method("total", &ModuleBrackets::total) + .field("value", &ModuleBrackets::value) + ; + + class_("ModuleBracketsOnly") + .constructor() + .method("[[", &ModuleBracketsOnly::at) + .method("[[", &ModuleBracketsOnly::at2) + .field("value", &ModuleBracketsOnly::value) + ; + class_("ModuleOwned") .field_readonly("id", &ModuleOwned::id) ; diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R index 59e1cb3f5..b4f394231 100644 --- a/inst/tinytest/test_module.R +++ b/inst/tinytest/test_module.R @@ -167,6 +167,14 @@ expect_error( r$get(10L), "not initialized" ) expect_error( Rcpp::sourceCpp("cpp/ModuleInitError.cpp"), "no such class" ) expect_true( Module_scope_is_cleared() ) +## completion lists the ordinary methods and the fields; an overloaded [[ +## used to be counted once per overload, which dropped the last ordinary +## method, or ran past the end of the method map when there was none (#1529) +brackets <- new( ModuleBrackets ) +expect_equal( Rcpp:::complete( brackets ), c("total( ", "value") ) +expect_equal( brackets[[2L]], 2.0 ) +expect_equal( Rcpp:::complete( new( ModuleBracketsOnly ) ), "value" ) + ## a pointer returned from C++ is owned by the R object wrapping it, and ## deleted when that object is garbage collected (#1533) destroyed <- ModuleOwned_destroyed()