Sugar operator+ and SubsetProxy

[nathan-russell]

The following fails to compile (x86_64-redhat-linux, g++ 4.8.3, R 3.2.2),

#include <Rcpp.h>
// [[Rcpp::export]]
Rcpp::NumericVector SubsetAdd(Rcpp::NumericVector lhs, Rcpp::NumericVector rhs) {
    Rcpp::LogicalVector idx = !Rcpp::is_na(lhs) & !Rcpp::is_na(rhs);
    return lhs[idx] + rhs[idx];        // error
} 

with error

no match for ‘operator+’ (operand types are 
‘Rcpp::SubsetProxy<14, Rcpp::PreserveStorage, 10, true, Rcpp::Vector<10, Rcpp::PreserveStorage> >’ 
and 
‘Rcpp::SubsetProxy<14, Rcpp::PreserveStorage, 10, true, Rcpp::Vector<10, Rcpp::PreserveStorage> >’)

This compiles and evaluates correctly:

Rcpp::NumericVector SubsetAdd(Rcpp::NumericVector lhs, Rcpp::NumericVector rhs) {
    Rcpp::LogicalVector idx = !Rcpp::is_na(lhs) & !Rcpp::is_na(rhs);

    return static_cast<const Rcpp::NumericVector&>(lhs[idx]) + 
        static_cast<const Rcpp::NumericVector&>(rhs[idx]);           // Ok
}

but surely is not ideal. I was unsuccessful in coming up with a fix for this, but hopefully someone can point me in the direction of a clever solution? (Presumably this functionality is desirable & possible.)

[eddelbuettel]

A somewhat casual observation from my usage is that the compiler has difficulties with complex and compound return statements. This more defensive variant works:

#include <Rcpp.h>
// [[Rcpp::export]]
Rcpp::NumericVector SubsetAdd(Rcpp::NumericVector lhs, Rcpp::NumericVector rhs) {
    Rcpp::LogicalVector idx = !Rcpp::is_na(lhs) & !Rcpp::is_na(rhs);
    Rcpp::NumericVector a = lhs[idx];
    Rcpp::NumericVector b = rhs[idx];
    return a + b;
} 

as seen here

R> library(Rcpp)
R> sourceCpp("/tmp/nrussell.cpp")
R> SubsetAdd(1:5, c(1,NA,3:5))
[1]  2  6  8 10
R> 

Good enough to close this? Fixing this would get pretty deep into the TMP bowels....

[nathan-russell]

Yes of course; I had a feeling this would involve some serious template wizardry, but was hoping I had overlooked some obvious quick fix. I'll close this out.

[kevinushey]

Reopening just because I think this should be possible + is worth doing.

[thirdwing]

Add something like below into Subsetter.h should be enough to fix this.

template <int RTYPE_OTHER, template <class> class StoragePolicyOther,int RHS_RTYPE_OTHER, bool RHS_NA_OTHER, typename RHS_T_OTHER>
SubsetProxy& operator+(const SubsetProxy<RTYPE_OTHER, StoragePolicyOther, RHS_RTYPE_OTHER, RHS_NA_OTHER, RHS_T_OTHER>& other) {
    if (other.indices_n == 1) {
        for (int i = 0; i < indices_n; ++i)
            lhs[ indices[i] ] += other.lhs[other.indices[0]];
    } else if (indices_n == other.indices_n) {
        for (int i = 0; i < indices_n; ++i)
            lhs[ indices[i] ] += other.lhs[other.indices[i]];
    } else {
        stop("index error");
    }
    return *this;
}

If we support +, then we should support other operators, right?

[kevinushey]

Indeed. I wonder if we could just generate all that code (either with a macro, or an R script, or otherwise).

[nathan-russell]

Well that was fast - and much more concise than I thought it would be. Very nice work guys.

[eddelbuettel]

You guys roll. I was out "socialising" on a Friday...