Module enhancements #454
Module enhancements #454
Conversation
…of a class
with the `copy_constructor` declaration:
```
#include <Rcpp.h>
using namespace Rcpp ;
class Foo{
public:
Foo( int x_ ) : x(x_){}
int x ;
} ;
RCPP_MODULE(test){
class_<Foo>("Foo")
.constructor<int>()
.copy_constructor()
.field( "x", &Foo::x)
;
}
```
This is associated with the `copy` R function :
```
f <- new( Foo, 1 )
g <- copy( f)
g$x
!identical( g$.pointer, f$.pointer )
```
|
One possible (trivial) enhancement would be to use the |
romainfrancois
changed the title
|
Looks good from a very first glance (provided of course we get it to pass Travis). I had called what I put through a full rev.dep 0.12.4.2 but had not committed that yet so we may as well got to 0.12.4.3. Once minor concern is |
|
From the raw log: I presume this builds on your OS X box? |
|
About passing tests. I'm working on it. About naming, not a big issue, I just want/need the functionality, so you can decide whatever name suits you best. I find |
|
I'm sure you'll have it working in no time. Happy to merge then. @jjallaire @kevinushey @thirdwing I'd love to hear additional views on naming of |
constructor is now automatic with the `has_copy_constructor` trait.
|
Now dropped the explicit declaration of the copy constructor, we can do this automatically with the |
|
Hmm. My SFINAE trick : does not seem to work on the compiler used in travis : |
|
I'm fine with calling the function Alternatively, Or just call it |
|
Oh, I forgot about I thought of |
|
What about making |
|
I don't mind at all about the naming. I just put in what made sense to me and I knew this would happen. @kevinushey you mean something like : That's definitely possible, I find this less visually pleasing, and also it means that the class can't have a genuine Anyway, whatever you decide. We might have the same argument later on when I introduce something to explicitly destruct (i.e. call the destructor) an object. I was thinking of @kevinushey do you have a suggestion for detecting if a class is copy constructible without getting all these compiler errors ? |
|
Not really :/ I can just share what we do in RStudio -- it's similar except we don't inherit from a base SFINAE class: #define RS_GENERATE_HAS_TYPE_TRAIT(__NAME__) \
template <typename T> struct has_##__NAME__##_impl \
{ \
template <typename U, typename V> struct SFINAE \
{ \
}; \
\
template <typename U> \
static char test(SFINAE<U, typename U::__NAME__>*); \
\
template <typename U> static int test(...); \
\
static const bool value = sizeof(test<T>(0)) == sizeof(char); \
}; \
\
template <typename T> \
struct has_##__NAME__ \
: public boost::integral_constant<bool, \
has_##__NAME__##_impl<T>::value> \
{ \
}
// metafunction for checking if class has 'key_type' type
RS_GENERATE_HAS_TYPE_TRAIT(key_type); |
|
yeah that's what we use for Note that it fails because of this class in test: Perhaps we could require classes to be copy constructible and hence drop the test. This test is not really relevant. |
|
I'm trying a different approach. #if defined(RCPP_USING_CXX11)
template <typename T> struct has_copy_constructor :
integral_constant<bool, std::is_copy_constructible<T>::value >{} ;
#else
template<typename T> struct has_copy_constructor : true_type {} ;
#endifSo essentially when using C++11, we use the namespace Rcpp {
namespace traits {
template <> struct has_copy_constructor<ModuleTest> : false_type {} ;
}
}or using the RCPP_DISABLE_COPY_CONSTRUCTOR(ModuleTest)I would have preferred a way without this extra declarative step but this is I think a good compromise, esp given that most of the classes exposable by modules will have some copy constructor, so this is a small price to pay for having the |
- explicitely destruct a C++ object (without waiting for the GC) - check if an object has been destructed
|
Also adding #include <Rcpp.h>
using namespace Rcpp ;
class Foo{
public:
Foo( int x_ ) : x(x_){}
~Foo(){
Rprintf( "~Foo\n" ) ;
}
int x ;
} ;
RCPP_MODULE(test){
class_<Foo>("Foo")
.constructor<int>()
.field( "x", &Foo::x)
;
}> foo <- new(Foo, 1)
> foo
C++ object <0x7fd353c910e0> of class 'Foo' <0x7fd353c14600>
> destruct(foo)
~Foo
> foo$.pointer
<pointer: 0x0>
> is_destructed(foo)
[1] TRUE |
|
Looks good. I think I would like Pinging @jjallaire @kevinushey @thirdwing |
|
I don't mind Rcpp::copy so much because I don't think there is much if any On Fri, Apr 1, 2016 at 7:56 AM, Dirk Eddelbuettel notifications@github.com
|
|
But if we export it in |
|
Okay, I get that. I wasn't thinking about calling sourceCpp in an Rcpp::copyObject Yuk, but clear and avoids the clash :-\ On Fri, Apr 1, 2016 at 8:18 AM, Dirk Eddelbuettel notifications@github.com
|
|
I'll probably deal with all that tomorrow morning. We can let it simmer til then. |
|
I can make the changes in the PR once you reach a decision. copyObject makes more sense to me than copyModule. Clearer what it means. Romain
|
|
Yes, please change from |
changes in Rcpp modules to allow use of the copy constructor of a class
This is associated with the
copyR function :