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


--
Romain Francois
Professional R Enthusiast
+33(0) 6 28 91 30 30
http://romainfrancois.blog.free.fr
|- http://bit.ly/9aKDM9 : embed images in Rd documents
|- http://tr.im/OIXN : raster images and RImageJ
|- http://tr.im/OcQe : Rcpp 0.7.7


_______________________________________________
Rcpp-devel mailing list
[email protected]
https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel

Reply via email to