Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RCPP_CTOR_ASSIGN compiler warning in Rcpp/Module.h #410

Closed
jpritikin opened this issue Dec 29, 2015 · 9 comments
Closed

RCPP_CTOR_ASSIGN compiler warning in Rcpp/Module.h #410

jpritikin opened this issue Dec 29, 2015 · 9 comments

Comments

@jpritikin
Copy link
Contributor

@jpritikin jpritikin commented Dec 29, 2015

I might be able to figure out the cause of this warning, but you could probably figure it out much faster.

/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/Module.h: In copy constructor ‘Rcpp::CppClass::CppClass(const Rcpp::CppClass&)’:
/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/Module.h:404:23: warning: base class ‘class Rcpp::S4_Impl<Rcpp::PreserveStorage>’ should be explicitly initialized in the copy constructor [-Wextra]
      RCPP_CTOR_ASSIGN(CppClass)
                       ^
/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/macros/interface.h:27:1: note: in definition of macro ‘RCPP_CTOR_ASSIGN’
 __CLASS__( const __CLASS__& other ){                                           \
 ^
/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/Module.h: In copy constructor ‘Rcpp::CppObject::CppObject(const Rcpp::CppObject&)’:
/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/Module.h:416:26: warning: base class ‘class Rcpp::S4_Impl<Rcpp::PreserveStorage>’ should be explicitly initialized in the copy constructor [-Wextra]
         RCPP_CTOR_ASSIGN(CppObject)
                          ^
/opt/R/x86_64-pc-linux-gnu-library/3.2/Rcpp/include/Rcpp/macros/interface.h:27:1: note: in definition of macro ‘RCPP_CTOR_ASSIGN’
 __CLASS__( const __CLASS__& other ){                                           \
 ^
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Dec 29, 2015

  1. Complete reproducible example, please, or it is no bug report or issue.
  2. What compiler? What OS? What version of things?
  3. Just add -Wno-extra to your compiler options to suppress this. It is a warning.
@jpritikin
Copy link
Contributor Author

@jpritikin jpritikin commented Dec 29, 2015

Rcpp 0.12.2, R 3.2.2, gcc (Tanglu 5.2.1-16) 5.2.1 20150903

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Dec 29, 2015

g++-5 is known to be pickier. Pull requests welcome.

@jpritikin
Copy link
Contributor Author

@jpritikin jpritikin commented Dec 29, 2015

I'm willing to spend some time to investigate. Can you give me a hint about what to fix?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Dec 29, 2015

Really appreciate this -- the 90k lines of code for Rcpp can be very convoluted and tough to decipher. I'd start by seeing what RCPP_CTOR_ASSIGN is made of and the try to find the copy constructor this warns about.

@jpritikin
Copy link
Contributor Author

@jpritikin jpritikin commented Dec 29, 2015

The warning is solved by changing this,

#define RCPP_CTOR_ASSIGN1(__CLASS__)                                           \
__CLASS__( const __CLASS__& other ){                                           \
    Storage::copy__(other) ;                                                   \
}                                                                              \

to this

#define RCPP_CTOR_ASSIGN(__CLASS__)                                            \
  __CLASS__( const __CLASS__& other ) : Base(other) {           \
    Storage::copy__(other) ;                                                   \
}                                                                              \

However, this requires (for the macro) that there is only 1 base class. For users of RCPP_GENERATE_CTOR_ASSIGN, the change is not needed. So maybe the way forward is to create 2 versions of RCPP_CTOR_ASSIGN, one with the Base(other) thing and one without.

Comments?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Dec 29, 2015

In that case we would need two distinct versions of RCPP_CTOR_ASSIGN as you can't have two with the same name; maybe keep the existing one as RCPP_CTOR_ASSIGN_ORIG or something. We could even flag it to be deprecated in 12+ months. All this assuming that reverse depends check don't reveal anything nasty.

@jpritikin
Copy link
Contributor Author

@jpritikin jpritikin commented Dec 29, 2015

Hm, it's it redundant to initialize the base class and also do Storage::copy__ ?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Jan 8, 2016

This can be closed as @thirdwing reminds us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.