count special module methods once per name for completion (#1530)

* count special module methods once per name for completion

Author: kevinushey

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

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 ;
         }
 

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>("ModuleBrackets")
+        .constructor()
+        .method("[[", &ModuleBrackets::at)
+        .method("[[", &ModuleBrackets::at2)
+        .method("total", &ModuleBrackets::total)
+        .field("value", &ModuleBrackets::value)
+        ;
+
+    class_<ModuleBracketsOnly>("ModuleBracketsOnly")
+        .constructor()
+        .method("[[", &ModuleBracketsOnly::at)
+        .method("[[", &ModuleBracketsOnly::at2)
+        .field("value", &ModuleBracketsOnly::value)
+        ;
+
     class_<ModuleOwned>("ModuleOwned")
         .field_readonly("id", &ModuleOwned::id)
         ;

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()