On Mon, May 10, 2010 at 8:32 AM, Romain Francois <[email protected]> wrote: > > Le 10/05/10 15:06, Romain Francois a écrit : >> >> Le 10/05/10 14:13, Romain Francois a écrit : >>> >>> Le 08/05/10 16:21, Douglas Bates a écrit : >>>> >>>> The enclosed section of code using Rcpp::Dimension fails to compile >>>> because of the const qualifiers for the simple::nrow and simple::ncol >>>> method functions. If you omit those const qualifiers then it will >>>> compile and behave as desired. Of course, I would prefer to have >>>> those method functions use the const qualifier. Does anyone have >>>> suggestions on how to modify this code so I can use the const >>>> qualifiers? >>>> >>>> My guess is that this is related to the Rcpp::Dimension::operator[]() >>>> returning an int& and not an int so somehow there is a delayed >>>> evaluation going on - but I haven't learned enough C++ to be able to >>>> decide exactly where the problem originates. >>> >>> Hmm. The compiler error is really weird. It is not related to an >>> underlying SEXP because there is no underlying SEXP. Rcpp::Dimension >>> stores its data in a std::vector<int> >>> >>> If I simplfy the code to this : >>> >>> library(Rcpp) >>> cdef <- ' >>> class simple { >>> Rcpp::Dimension dd; >>> public: >>> simple() : dd(1, 3) {} >>> int nrow() const { return dd[0] ; } >>> }; >>> ' >>> cpp <- ' >>> simple ss; >>> return wrap( ss.nrow() ) ; >>> ' >>> ff <- cppfunction(signature(), cpp, includes = cdef) >>> ff( ) >>> >>> >>> I get this compiler error: >>> >>> Le chargement a nécessité le package : inline >>> Le chargement a nécessité le package : methods >>> file10d63af1.cpp: In member function ‘int simple::nrow() const’: >>> file10d63af1.cpp:8: error: invalid use of incomplete type ‘struct >>> SEXPREC’ >>> /Library/Frameworks/R.framework/Resources/include/Rinternals.h:333: >>> error: forward declaration of ‘struct SEXPREC’ >>> file10d63af1.cpp:8: error: cannot convert ‘SEXPREC’ to ‘int’ in return >>> make: *** [file10d63af1.o] Error 1 >>> >>> >>> which is __really__ weird... (time passes)... I think it comes from the >>> operator SEXP. The compiler decided, for some reason to convert the >>> Rcpp::Dimension to a SEXP using operator SEXP, then add 0 and >>> dereference: >>> >>> so dd[0] becomes *( dd + 0 ) which becomes *( (SEXP)dd + 0 ) and I guess >>> this is where it gets unhappy. >>> >>> >>> If I change the code to explicitely call the operator[]: >>> >>> int nrow() const { return dd.operator[](0) ; } >>> >>> I then get a much more reasonnable compiler error: >>> >>> file10d63af1.cpp: In member function ‘int simple::nrow() const’: >>> file10d63af1.cpp:8: error: passing ‘const Rcpp::Dimension’ as ‘this’ >>> argument of ‘int& Rcpp::Dimension::operator[](int)’ discards qualifiers >>> make: *** [file10d63af1.o] Error 1 >>> >>> >>> I am not sure why the compiler does not want to call the operator[], so >>> there is probably something we miss in Rcpp::Dimension. >>> >>> Also, Rcpp::Dimension objects are often used temporarily (in matrix >>> constructors, etc ...) so maybe we don't need them to return references >>> and can return int instead. If you want to play with changing semantics >>> of Rcpp::Dimension so that it returns "int" instead of "int&", that's >>> fine by me. at the moment I don't think we do anything like: >>> >>> Rcpp::Dimension x(3,3) ; >>> x[0] = 4 ; >>> >>> so we don't need references here. >>> >>> >>> We might still need two versions of operator[] to deal with the >>> eccentric decision from the compiler. >>> >>> Romain >> >> I get it now. Your code compiles after the attached patch is applied ( I >> can't do it right now because r-forge is full and won't let me commit). > > Seems to be back up now. So I've commited it. > >> The compiler is not eccentric, it just knows the rule better than we do. >> Since the operator[] is not const, the compiler does not want it when >> doing dd[0], then it looks at alternative and has the idea to cast the >> Dimension to a SEXP and dereference that after. >> >> The patch adds a const operator[]: >> >> typedef std::vector<int>::reference reference ; >> typedef std::vector<int>::const_reference const_reference ; >> reference operator[](int i) throw(std::range_error) ; >> const_reference operator[](int i) const throw(std::range_error) ; >> >> which suits your code, but keeps the reference semantics so that we >> don't break any code that would use them. >> >> I think there are more examples like this in one of Scott Meyers's books >> (effective c++, ...), about how we sometimes need const and non const >> versions of operator[]. >> >> Romain > > > --
As always, thanks very much for going above and beyond in responding to questions. I really appreciate your looking at this in such detail. _______________________________________________ Rcpp-devel mailing list [email protected] https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
