cppFunction won't compile

[tjmckinley]

Hi,
There is an issue with cppFunction in that it won't compile if you require multiple libraries. A minimal reproducible example is below. (I appreciate that this example doesn't actually require the BH and RcppArmadillo header files, but the presence of the former means that the header file for latter isn't linked.)

library(Rcpp)
cppFunction('arma::mat timesTwo(arma::mat x) {
    return x * 2.0;}', depends = c("BH", "RcppArmadillo"))

Running the above example causes a compilation error. I have raised a pull request (#1016 ) that fixes this. The result of me running sessionInfo() is:

R version 3.5.2 (2018-12-20)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 19.04

Matrix products: default
BLAS: /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.8.0
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.8.0

locale:
 [1] LC_CTYPE=en_GB.UTF-8       LC_NUMERIC=C               LC_TIME=en_GB.UTF-8        LC_COLLATE=en_GB.UTF-8     LC_MONETARY=en_GB.UTF-8   
 [6] LC_MESSAGES=en_GB.UTF-8    LC_PAPER=en_GB.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] Rcpp_1.0.3

loaded via a namespace (and not attached):
[1] compiler_3.5.2            tools_3.5.2               RcppArmadillo_0.9.800.1.0 BH_1.69.0-1 

Many thanks,

TJ

[eddelbuettel]

Oh, I see now. At least you have a bug. Maybe.

I think what you are hitting is not related to how the #include unwind. You may have hitten the issue of how Rcpp and RcppArmadillo hitters are intertwined.

A counterexample is

R> Rcpp::cppFunction('arma::mat timesThree(arma::mat x) {return x * 3.0;}', depends = c("RcppArmadillo","BH"))
R> timesThree(matrix(c(1,7,14),3))
     [,1]
[1,]    3
[2,]   21
[3,]   42
R> 

[tjmckinley]

I think it's more fundamental thatn that. If you run with the verbose option turned on you can see that the BH.h header file is not linked in the code at all (I think)

> Rcpp::cppFunction('arma::mat timesThree(arma::mat x) {return x * 3.0;}', depends = c("RcppArmadillo", "BH"), verbose = T)

Generated code for function definition: 
--------------------------------------------------------

// [[Rcpp::depends(RcppArmadillo)]]
// [[Rcpp::depends(BH)]]

#include <RcppArmadillo.h>
#include <Rcpp.h>

using namespace Rcpp;

// [[Rcpp::export]]
arma::mat timesThree(arma::mat x) {return x * 3.0;}

[eddelbuettel]

I looked at verbose too. That would be a bug. But if you read ?cppFunction then you see that depends=... is for plugins and BH does not have one. Then again piling in RcppGSL and RcppArmadillo does lead to failure depending on the order.

But finally: it is cppFunction. You are not supposed to write a magnus opus with it. We use sourceCpp for more-than-trivial tasks, and packages for real work.

Then again, what we have are misfeatures/bugs. If you PR makes these better, then I am all for it. Will take another look later. Gotta get out for a run now but back later.

Thanks for following up with code to discuss. Much prefered over a naked PR.

[tjmckinley]

Of course, no worries. And apologies for leaving a naked PR in the first place. Thanks for looking at this further and for the detailed reply. For context, I am writing an R package that uses cppFunction within it to produce compiled objects "on demand". I have a few custom Rcpp functions that I want to make available to these "on-the-fly" functions using the "Rcpp interfaces" options. I could do this using sourceCppso happy to change that if I need to, but it's tidier using cppFunction.

What made me think it was a bug was that in the source code for cppFunction there were statements of the form vector <- paste(vector, sep = ", "), which doesn't actually change vector in any way. However, vector <- paste(vector, collapse = ", ") does e.g.

> paste(c("x", "y"), sep = ", ")
[1] "x" "y"
> paste(c("x", "y"), collapse = ", ")
[1] "x, y"

so I assumed the latter was intended, else the paste() statement is redundant (I think).

Many thanks once again,

TJ

[eddelbuettel]

I'm on board now.

It is an issue you correctly identified and fixed -- so thanks. (Minor nit: ChangeLog entries are welcome too. Easiest in Emacs via C-x 4 a, or copy & paste and edit in other editors.)

[tjmckinley]

Perfect. Many thanks. Apologies for not changing the ChangeLog - I'm still feeling my way through how to contribute in the right way. I'm used to coding by myself. Many thanks.

[eddelbuettel]

No worries. Approximately nobody [1] adheres to my suggestion to keep a ChangeLog. If you do, great, if you don't I [sighs audibly] just do it myself ;-)

[1] Slight exaggeration. Some people do.