Variadic templates refinement (#1336)

* add tests to check that void functions return invisibly * uncomment HAS_VARIADIC_TEMPLATES capability, simplify ifdefs * add missing is_void method to CppFunctionN * update ChangeLog
RcppCore · Oct 7, 2024 · c91eb89 · c91eb89
1 parent 982f784
commit c91eb89
Show file tree

Hide file tree

Showing 17 changed files with 70 additions and 24 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,23 @@
+2024-10-07  Iñaki Ucar  <iucar@fedoraproject.org>
+
+	* inst/include/Rcpp/platform/compiler.h: Uncomment HAS_VARIADIC_TEMPLATES
+	macro definition
+	* src/api.cpp: Simplify checks for variadic templates
+	* inst/include/Rcpp/DataFrame.h: Idem
+	* inst/include/Rcpp/DottedPair.h: Idem
+	* inst/include/Rcpp/Function.h: Idem
+	* inst/include/Rcpp/InternalFunctionWithStdFunction.h: Idem
+	* inst/include/Rcpp/Language.h: Idem
+	* inst/include/Rcpp/Pairlist.h: Idem
+	* inst/include/Rcpp/grow.h: Idem
+	* inst/include/Rcpp/internal/call.h: Idem
+	* inst/include/Rcpp/module/class.h: Idem
+	* inst/include/Rcpp/traits/index_sequence.h: Idem
+	* inst/include/Rcpp/traits/named_object.h: Idem
+	* inst/include/Rcpp/vector/Vector.h: Idem
+	* inst/include/Rcpp/Module.h: Idem + add missing is_void method
+	* inst/tinytest/test_module.R: Add test for void functions and methods
+
 2024-10-04  Dirk Eddelbuettel  <edd@debian.org>
 
 	* DESCRIPTION (Version, Date): Roll micro version to 1.0.13.3

diff --git a/inst/include/Rcpp/DataFrame.h b/inst/include/Rcpp/DataFrame.h
@@ -117,7 +117,7 @@ namespace Rcpp{
         static DataFrame_Impl create(){
             return DataFrame_Impl() ;
         }
-        #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+        #if defined(HAS_VARIADIC_TEMPLATES)
             template <typename... T>
             static DataFrame_Impl create(const T&... args) {
                 return DataFrame_Impl::from_list(Parent::create(args...));

diff --git a/inst/include/Rcpp/DottedPair.h b/inst/include/Rcpp/DottedPair.h
@@ -35,7 +35,7 @@ RCPP_API_CLASS(DottedPair_Impl),
 	DottedPair_Impl(SEXP x) {
 	    Storage::set__(x) ;
 	}
-  #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+  #if defined(HAS_VARIADIC_TEMPLATES)
 		template <typename... T>
 		DottedPair_Impl(const T&... args) {
 			Storage::set__(pairlist(args...));

diff --git a/inst/include/Rcpp/Function.h b/inst/include/Rcpp/Function.h
@@ -82,7 +82,7 @@ namespace Rcpp{
             return Rcpp_fast_eval(call, R_GlobalEnv);
         }
 
-        #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+        #if defined(HAS_VARIADIC_TEMPLATES)
             template <typename... T>
             SEXP operator()(const T&... args) const {
             return invoke(pairlist(args...), R_GlobalEnv);

diff --git a/inst/include/Rcpp/InternalFunctionWithStdFunction.h b/inst/include/Rcpp/InternalFunctionWithStdFunction.h
@@ -23,7 +23,7 @@
 #ifndef Rcpp_InternalFunctionWithStdFunction_h
 #define Rcpp_InternalFunctionWithStdFunction_h
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
 #include <Rcpp/internal/call.h>
 #endif
 #include <functional>

diff --git a/inst/include/Rcpp/Language.h b/inst/include/Rcpp/Language.h
@@ -102,7 +102,7 @@ namespace Rcpp{
          * 0.0 is wrapped as a numeric vector using wrap( const& double )
          * ...
          */
-        #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+        #if defined(HAS_VARIADIC_TEMPLATES)
             template <typename... T>
             Language_Impl(const std::string& symbol, const T&... t) {
                 Storage::set__(pairlist(Rf_install(symbol.c_str()), t...) );

diff --git a/inst/include/Rcpp/Module.h b/inst/include/Rcpp/Module.h
@@ -86,7 +86,7 @@ namespace Rcpp{
 #include <Rcpp/module/CppFunction.h>
 #include <Rcpp/module/get_return_type.h>
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
 namespace Rcpp {
     template <typename RESULT_TYPE, typename... T>
     inline void signature(std::string& s, const char* name) {
@@ -112,6 +112,7 @@ namespace Rcpp {
             }
 
             inline int nargs() { return sizeof...(T); }
+            inline bool is_void() { return std::is_void<RESULT_TYPE>::value; }
             inline void signature(std::string& s, const char* name) { Rcpp::signature<RESULT_TYPE, T...>(s, name); }
             inline DL_FUNC get_function_ptr() { return (DL_FUNC)ptr_fun; }
 
@@ -176,7 +177,7 @@ namespace Rcpp{
     private:
         ParentMethod* parent_method_pointer ;
     } ;
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
     template <typename... T>
     inline void ctor_signature(std::string& s, const std::string& classname) {
         s.assign(classname);
@@ -380,7 +381,7 @@ namespace Rcpp{
 
     } ;
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
     template <bool IsConst,typename Class, typename RESULT_TYPE, typename... T>
     class CppMethodImplN : public CppMethod<Class> {
     public:
@@ -428,7 +429,7 @@ namespace Rcpp{
             return call<decltype(f), CLEANED_RESULT_TYPE, T...>(f, args);
         }
         inline int nargs() { return sizeof...(T); }
-        inline bool is_void() { return std::is_void<RESULT_TYPE>::value;}
+        inline bool is_void() { return std::is_void<RESULT_TYPE>::value; }
         inline bool is_const() { return IsConst; }
         inline void signature(std::string& s, const char* name) { Rcpp::signature<RESULT_TYPE,T...>(s, name); }
     private:
@@ -551,7 +552,7 @@ namespace Rcpp{
     } ;
 }
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
 namespace Rcpp {
     template <typename RESULT_TYPE, typename... T>
     void function(const char* name_,  RESULT_TYPE (*fun)(T... t), const char* docstring = 0) {

diff --git a/inst/include/Rcpp/Pairlist.h b/inst/include/Rcpp/Pairlist.h
@@ -42,7 +42,7 @@ namespace Rcpp{
         Pairlist_Impl(SEXP x){
             Storage::set__(r_cast<LISTSXP>(x)) ;
         }
-        #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+        #if defined(HAS_VARIADIC_TEMPLATES)
             template <typename... T>
             Pairlist_Impl(const T&... args ){
                 Storage::set__(pairlist(args... )) ;

diff --git a/inst/include/Rcpp/grow.h b/inst/include/Rcpp/grow.h
@@ -70,7 +70,7 @@ namespace Rcpp {
         return grow(Rf_mkString(head), y);
     }
 
-    #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+    #if defined(HAS_VARIADIC_TEMPLATES)
         template <typename T1>
         SEXP pairlist(const T1& t1) {
             return grow( t1, R_NilValue ) ;

diff --git a/inst/include/Rcpp/internal/call.h b/inst/include/Rcpp/internal/call.h
@@ -4,7 +4,7 @@
 #include <Rcpp/traits/index_sequence.h>
 #include <functional>
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
 
 namespace Rcpp {
 namespace internal {
@@ -15,7 +15,7 @@ template <typename... T> struct type_pack {};
 /**
  * This specialisation is for functions that return a value, whereas the below
  * is for void-returning functions.
- * 
+ *
  * The "* = nullptr" default argument allows both templates to be well-defined
  * regardless of which one is used.
  */
@@ -40,7 +40,7 @@ SEXP call_impl(const F& fun, SEXP* args, type_pack<RESULT_TYPE, Us...>,
  * Helper for calling a function with an array of SEXP arguments,
  * where each argument is converted to the appropriate type before being passed
  * to the function. A compile-time sequence is used to index the SEXP array.
- * 
+ *
  * The function only needs the intended types of the result and arguments,
  *  which allows the template to be used for function pointers, lambdas, and
  * `std::function` objects.

diff --git a/inst/include/Rcpp/module/class.h b/inst/include/Rcpp/module/class.h
@@ -110,7 +110,7 @@
             return constructor( docstring, valid ) ;
         }
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
     template <typename... T>
     self& constructor( const char* docstring = 0, ValidConstructor valid = &yes_arity<sizeof...(T)> ){
         AddConstructor( new Constructor<Class,T...> , valid, docstring ) ;
@@ -265,7 +265,7 @@
             return *this ;
         }
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
     template <typename RESULT_TYPE, typename... T>
     self& method(const char* name_, RESULT_TYPE (Class::*fun)(T...),
                 const char* docstring = 0, ValidMethod valid = &yes_arity<sizeof...(T)>) {

diff --git a/inst/include/Rcpp/platform/compiler.h b/inst/include/Rcpp/platform/compiler.h
@@ -68,7 +68,7 @@
     #if __cplusplus >= 201103L
         #define RCPP_USING_CXX11
         #if __INTEL_COMPILER >= 1210
-            // #define HAS_VARIADIC_TEMPLATES
+            #define HAS_VARIADIC_TEMPLATES
         #endif
         #if __INTEL_COMPILER >= 1100
             #define HAS_STATIC_ASSERT
@@ -78,7 +78,7 @@
     #if __cplusplus >= 201103L
         #define RCPP_USING_CXX11
         #if __has_feature(cxx_variadic_templates)
-            // #define HAS_VARIADIC_TEMPLATES
+            #define HAS_VARIADIC_TEMPLATES
         #endif
         #if __has_feature(cxx_static_assert)
             #define HAS_STATIC_ASSERT
@@ -87,7 +87,7 @@
 #elif defined(__GNUC__)
     #ifdef __GXX_EXPERIMENTAL_CXX0X__
         #if GCC_VERSION >= 40300
-            // #define HAS_VARIADIC_TEMPLATES
+            #define HAS_VARIADIC_TEMPLATES
             #define HAS_STATIC_ASSERT
         #endif
     #endif

diff --git a/inst/include/Rcpp/traits/index_sequence.h b/inst/include/Rcpp/traits/index_sequence.h
@@ -2,7 +2,7 @@
 #define RCPP_TRAITS_INDEX_SEQUENCE_H
 
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
 
 namespace Rcpp {
 namespace traits {

diff --git a/inst/include/Rcpp/traits/named_object.h b/inst/include/Rcpp/traits/named_object.h
@@ -66,7 +66,7 @@ template <typename T> struct is_named< named_object<T> > : public true_type {};
 template <> struct is_named< Rcpp::Argument > : public true_type {};
 
 
-#if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+#if defined(HAS_VARIADIC_TEMPLATES)
     template <typename... T> struct is_any_named : public false_type {};
     template <typename T> struct is_any_named<T> : public is_named<T>::type {};
 

diff --git a/inst/include/Rcpp/vector/Vector.h b/inst/include/Rcpp/vector/Vector.h
@@ -1122,7 +1122,7 @@ class Vector :
         return Vector( 0 ) ;
     }
 
-    #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+    #if defined(HAS_VARIADIC_TEMPLATES)
         public:
             template <typename... T>
             static Vector create(const T&... t){

diff --git a/inst/tinytest/test_module.R b/inst/tinytest/test_module.R
@@ -22,11 +22,25 @@ if( ! Rcpp:::capabilities()[["Rcpp modules"]] ) exit_file("Skipping as no Module
 
 Rcpp::sourceCpp("cpp/Module.cpp")
 
+# checks the presence of "invisible", added when the function returns void
+is_void <- function(call) {
+    if (length(grep("invisible", deparse(call))) == 0)
+        return(FALSE)
+    return(TRUE)
+}
+
 #    test.Module <- function(){
 expect_equal( bar( 2L ), 4L )
 expect_equal( foo( 2L, 10.0 ), 20.0 )
 expect_equal( hello(), "hello" )
 
+expect_false(is_void(bar@.Data))
+expect_false(is_void(foo@.Data))
+expect_false(is_void(hello@.Data))
+expect_true(is_void(bla@.Data))
+expect_true(is_void(bla1@.Data))
+expect_true(is_void(bla2@.Data))
+
 w <- new( ModuleWorld )
 expect_equal( w$greet(), "hello" )
 w$set( "hello world" )
@@ -38,13 +52,24 @@ expect_equal( w$greet(), "hello world const ref" )
 w$clear( )
 expect_equal( w$greet(), "" )
 
+expect_false(is_void(w$greet))
+expect_true(is_void(w$set))
+expect_true(is_void(w$set_ref))
+expect_true(is_void(w$set_const_ref))
+expect_true(is_void(w$clear))
+
 #    test.Module.exposed.class <- function(){
 test <- new( ModuleTest, 3.0 )
 expect_equal( Test_get_x_const_ref(test), 3.0 )
 expect_equal( Test_get_x_const_pointer(test), 3.0 )
 expect_equal( Test_get_x_ref(test), 3.0 )
 expect_equal( Test_get_x_pointer(test), 3.0 )
 
+expect_false(is_void(Test_get_x_const_ref@.Data))
+expect_false(is_void(Test_get_x_const_pointer@.Data))
+expect_false(is_void(Test_get_x_ref@.Data))
+expect_false(is_void(Test_get_x_pointer@.Data))
+
 expect_equal( attr_Test_get_x_const_ref(test), 3.0 )
 expect_equal( attr_Test_get_x_const_pointer(test), 3.0 )
 expect_equal( attr_Test_get_x_ref(test), 3.0 )

diff --git a/src/api.cpp b/src/api.cpp
@@ -257,7 +257,7 @@ SEXP rcpp_capabilities() {
 
 // [[Rcpp::internal]]
 SEXP rcpp_can_use_cxx0x() {                                     // #nocov start
-    #if defined(HAS_VARIADIC_TEMPLATES) || defined(RCPP_USING_CXX11)
+    #if defined(HAS_VARIADIC_TEMPLATES)
         return Rf_ScalarLogical(TRUE);
     #else
         return Rf_ScalarLogical(FALSE);