Thanks, that's really useful. One more question for you, or someone
else here:
const ArrayXd glmLink::linkFun(const ArrayXd& mu) const {
return as<ArrayXd>(::Rf_eval(::Rf_lang2(as<SEXP>(d_linkFun),
as<SEXP>(Rcpp::NumericVector(mu.data(),
mu.data() + mu.size()))
), d_rho);
}
I guess I need that to read
PROTECT(::Rf_eval(PROTECT(::Rf_lang2(...),...) , but as written it
doesn't seem I have anywhere to squeeze in an UNPROTECT(2). Do I need
to define a temporary variable so I can UNPROTECT(2) before I return the
value?
Or is there a way I can use Shield() since this an Rcpp-based project
anyway?
Sorry for all the very basic questions, but I'm flying nearly blind
here ...
cheers
Ben Bolker
On 2020-03-23 4:01 p.m., Tomas Kalibera wrote:> On 3/23/20 8:39 PM, Ben Bolker wrote:
>> Dear r-devel folks,
>>
>> ?? [if this is more appropriate for r-pkg-devel please let me know and
>> I'll repost it over there ...]
>>
>> I'm writing to ask for help with some R/C++ integration idioms that
are
>> used in a package I'm maintaining, that are unfamilar to me, and
that
>> are now being flagged as problematic by Tomas Kalibera's
'rchk'
>> machinery (https://github.com/kalibera/rchk); results are here
>>
https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/lme4.out
>>
>>
>> The problem is with constructions like
>>
>> ::Rf_eval(::Rf_lang2(fun, arg), d_rho)
>>
>> I *think* this means "construct a two-element pairlist from fun
and arg,
>> then evaluate it within expression d_rho"
>>
>> This leads to warnings like
>>
>> "calling allocating function Rf_eval with argument allocated using
>> Rf_lang2"
>>
>> Is this a false positive or ... ? Can anyone help interpret this?
> This is a true error. You need to protect the argument of eval() before
> calling eval, otherwise eval() could destroy it before using it. This is
> a common rule: whenever passing an argument to a function, that argument
> must be protected (directly or indirectly). Rchk tries to be smart and
> doesn't report a warning when it can be sure that in that particular
> case, for that particular function, it is safe. This is easy to fix,
> just protect the result of lang2() before the call and unprotect (some
> time) after.
>> Not sure why this idiom was used in the first place: speed? (e.g., see
>> https://stat.ethz.ch/pipermail/r-devel/2019-June/078020.html ) Should I
>> be rewriting to avoid Rf_eval entirely in favor of using a Function?
>> (i.e., as commented in
>>
https://stackoverflow.com/questions/37845012/rcpp-function-slower-than-rf-eval
>>
>> : "Also, calling Rf_eval() directly from a C++ context is
dangerous as R
>> errors (ie, C longjmps) will bypass the destructors of C++ objects and
>> leak memory / cause undefined behavior in general. Rcpp::Function tries
>> to make sure that doesn't happen.")
>
> Yes, eval (as well as lang2) can throw an error, this error has to be
> caught via R API and handled (e.g. by throwing as exception or something
> else, indeed that exception then needs to be caught and possibly
> converted back when leaving again to C stack frames). An R/C API you can
> use here is R_UnwindProtect. This is of course a bit of a pain, and one
> does not have to worry when programming in plain C.
>
> I suppose Rcpp provides some wrapper around R_UnwindProtect, that would
> be a question for Rcpp experts/maintainers.
>
> Best
> Tomas
>
>>
>> ? Any tips, corrections, pointers to further documentation, etc. would
be
>> most welcome ... Web searching for this stuff hasn't gotten me very
far,
>> and it seems to be deeper than most of the introductory material I can
>> find (including the Rcpp vignettes) ...
>>
>> ?? cheers
>> ??? Ben Bolker
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
Martin Maechler
2020-Mar-23 21:17 UTC
[Rd] help with rchk warnings on Rf_eval(Rf_lang2(...))
>>>>> Ben Bolker >>>>> on Mon, 23 Mar 2020 17:07:36 -0400 writes:> Thanks, that's really useful. One more question for you, or someone > else here: > const ArrayXd glmLink::linkFun(const ArrayXd& mu) const { > return as<ArrayXd>(::Rf_eval(::Rf_lang2(as<SEXP>(d_linkFun), > as<SEXP>(Rcpp::NumericVector(mu.data(), > mu.data() + mu.size())) > ), d_rho); > } > I guess I need that to read > PROTECT(::Rf_eval(PROTECT(::Rf_lang2(...),...) , but as written it > doesn't seem I have anywhere to squeeze in an UNPROTECT(2). Do I need > to define a temporary variable so I can UNPROTECT(2) before I return the > value? Ben, as co-author of the package, I could try .. I have lots of experience with (nice and clean) C API using PROTECT ... which I could try to apply here. I'm busy teaching tomorrow (with the extra time of setting up remote teaching ..) but could look into it later {and try using non-Rcpp C code}. > Or is there a way I can use Shield() since this an Rcpp-based project > anyway? > Sorry for all the very basic questions, but I'm flying nearly blind > here ... > cheers > Ben Bolker > On 2020-03-23 4:01 p.m., Tomas Kalibera wrote: >> On 3/23/20 8:39 PM, Ben Bolker wrote: >>> Dear r-devel folks, >>> >>> ?? [if this is more appropriate for r-pkg-devel please let me know and >>> I'll repost it over there ...] >>> >>> I'm writing to ask for help with some R/C++ integration idioms that are >>> used in a package I'm maintaining, that are unfamilar to me, and that >>> are now being flagged as problematic by Tomas Kalibera's 'rchk' >>> machinery (https://github.com/kalibera/rchk); results are here >>> https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/lme4.out >>> >>> >>> The problem is with constructions like >>> >>> ::Rf_eval(::Rf_lang2(fun, arg), d_rho) >>> >>> I *think* this means "construct a two-element pairlist from fun and arg, >>> then evaluate it within expression d_rho" >>> >>> This leads to warnings like >>> >>> "calling allocating function Rf_eval with argument allocated using >>> Rf_lang2" >>> >>> Is this a false positive or ... ? Can anyone help interpret this? >> This is a true error. You need to protect the argument of eval() before >> calling eval, otherwise eval() could destroy it before using it. This is >> a common rule: whenever passing an argument to a function, that argument >> must be protected (directly or indirectly). Rchk tries to be smart and >> doesn't report a warning when it can be sure that in that particular >> case, for that particular function, it is safe. This is easy to fix, >> just protect the result of lang2() before the call and unprotect (some >> time) after. >>> Not sure why this idiom was used in the first place: speed? (e.g., see >>> https://stat.ethz.ch/pipermail/r-devel/2019-June/078020.html ) Should I >>> be rewriting to avoid Rf_eval entirely in favor of using a Function? >>> (i.e., as commented in >>> https://stackoverflow.com/questions/37845012/rcpp-function-slower-than-rf-eval >>> >>> : "Also, calling Rf_eval() directly from a C++ context is dangerous as R >>> errors (ie, C longjmps) will bypass the destructors of C++ objects and >>> leak memory / cause undefined behavior in general. Rcpp::Function tries >>> to make sure that doesn't happen.") >> >> Yes, eval (as well as lang2) can throw an error, this error has to be >> caught via R API and handled (e.g. by throwing as exception or something >> else, indeed that exception then needs to be caught and possibly >> converted back when leaving again to C stack frames). An R/C API you can >> use here is R_UnwindProtect. This is of course a bit of a pain, and one >> does not have to worry when programming in plain C. >> >> I suppose Rcpp provides some wrapper around R_UnwindProtect, that would >> be a question for Rcpp experts/maintainers. >> >> Best >> Tomas >> >>> >>> ? Any tips, corrections, pointers to further documentation, etc. would be >>> most welcome ... Web searching for this stuff hasn't gotten me very far, >>> and it seems to be deeper than most of the introductory material I can >>> find (including the Rcpp vignettes) ... >>> >>> ?? cheers >>> ??? Ben Bolker >>> >>> ______________________________________________ >>> R-devel at r-project.org mailing list >>> https://stat.ethz.ch/mailman/listinfo/r-devel >> >> > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel
Simon Urbanek
2020-Mar-23 21:54 UTC
[Rd] help with rchk warnings on Rf_eval(Rf_lang2(...))
Ben, yes, you have to store the result into a variable, then unprotect, then return. Cheers, S> On 24/03/2020, at 10:07 AM, Ben Bolker <bbolker at gmail.com> wrote: > > > Thanks, that's really useful. One more question for you, or someone > else here: > > const ArrayXd glmLink::linkFun(const ArrayXd& mu) const { > return as<ArrayXd>(::Rf_eval(::Rf_lang2(as<SEXP>(d_linkFun), > > as<SEXP>(Rcpp::NumericVector(mu.data(), > > mu.data() + mu.size())) > ), d_rho); > } > > > I guess I need that to read > PROTECT(::Rf_eval(PROTECT(::Rf_lang2(...),...) , but as written it > doesn't seem I have anywhere to squeeze in an UNPROTECT(2). Do I need > to define a temporary variable so I can UNPROTECT(2) before I return the > value? > > Or is there a way I can use Shield() since this an Rcpp-based project > anyway? > > Sorry for all the very basic questions, but I'm flying nearly blind > here ... > > cheers > Ben Bolker > > > > On 2020-03-23 4:01 p.m., Tomas Kalibera wrote: >> On 3/23/20 8:39 PM, Ben Bolker wrote: >>> Dear r-devel folks, >>> >>> [if this is more appropriate for r-pkg-devel please let me know and >>> I'll repost it over there ...] >>> >>> I'm writing to ask for help with some R/C++ integration idioms that are >>> used in a package I'm maintaining, that are unfamilar to me, and that >>> are now being flagged as problematic by Tomas Kalibera's 'rchk' >>> machinery (https://github.com/kalibera/rchk); results are here >>> https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/lme4.out >>> >>> >>> The problem is with constructions like >>> >>> ::Rf_eval(::Rf_lang2(fun, arg), d_rho) >>> >>> I *think* this means "construct a two-element pairlist from fun and arg, >>> then evaluate it within expression d_rho" >>> >>> This leads to warnings like >>> >>> "calling allocating function Rf_eval with argument allocated using >>> Rf_lang2" >>> >>> Is this a false positive or ... ? Can anyone help interpret this? >> This is a true error. You need to protect the argument of eval() before >> calling eval, otherwise eval() could destroy it before using it. This is >> a common rule: whenever passing an argument to a function, that argument >> must be protected (directly or indirectly). Rchk tries to be smart and >> doesn't report a warning when it can be sure that in that particular >> case, for that particular function, it is safe. This is easy to fix, >> just protect the result of lang2() before the call and unprotect (some >> time) after. >>> Not sure why this idiom was used in the first place: speed? (e.g., see >>> https://stat.ethz.ch/pipermail/r-devel/2019-June/078020.html ) Should I >>> be rewriting to avoid Rf_eval entirely in favor of using a Function? >>> (i.e., as commented in >>> https://stackoverflow.com/questions/37845012/rcpp-function-slower-than-rf-eval >>> >>> : "Also, calling Rf_eval() directly from a C++ context is dangerous as R >>> errors (ie, C longjmps) will bypass the destructors of C++ objects and >>> leak memory / cause undefined behavior in general. Rcpp::Function tries >>> to make sure that doesn't happen.") >> >> Yes, eval (as well as lang2) can throw an error, this error has to be >> caught via R API and handled (e.g. by throwing as exception or something >> else, indeed that exception then needs to be caught and possibly >> converted back when leaving again to C stack frames). An R/C API you can >> use here is R_UnwindProtect. This is of course a bit of a pain, and one >> does not have to worry when programming in plain C. >> >> I suppose Rcpp provides some wrapper around R_UnwindProtect, that would >> be a question for Rcpp experts/maintainers. >> >> Best >> Tomas >> >>> >>> Any tips, corrections, pointers to further documentation, etc. would be >>> most welcome ... Web searching for this stuff hasn't gotten me very far, >>> and it seems to be deeper than most of the introductory material I can >>> find (including the Rcpp vignettes) ... >>> >>> cheers >>> Ben Bolker >>> >>> ______________________________________________ >>> R-devel at r-project.org mailing list >>> https://stat.ethz.ch/mailman/listinfo/r-devel >> >> > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >
Dirk Eddelbuettel
2020-Mar-23 21:55 UTC
[Rd] help with rchk warnings on Rf_eval(Rf_lang2(...))
On 23 March 2020 at 17:07, Ben Bolker wrote: | Or is there a way I can use Shield() since this an Rcpp-based project | anyway? Yes you can, and I would recommend it. Example from Rcpp itself, file Environment.h: Shield<SEXP> res(Rcpp_fast_eval(Rf_lang2(asEnvironmentSym, x), R_GlobalEnv)); For Rcpp_fast_eval, you may still need to #define RCPP_USE_UNWIND_PROTECT before including Rcpp.h; as I recall we were seeing too many side effects in other packages when globally enabling it. Or you can play it safe and simply use Rcpp_eval. This really was more of a rcpp-devel question. Dirk -- http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
Romain Francois
2020-Mar-24 10:33 UTC
[Rd] help with rchk warnings on Rf_eval(Rf_lang2(...))
> Le 23 mars 2020 ? 22:55, Dirk Eddelbuettel <edd at debian.org> a ?crit : > > On 23 March 2020 at 17:07, Ben Bolker wrote: > | Or is there a way I can use Shield() since this an Rcpp-based project > | anyway? > > Yes you can, and I would recommend it. > > Example from Rcpp itself, file Environment.h: > > Shield<SEXP> res(Rcpp_fast_eval(Rf_lang2(asEnvironmentSym, x), R_GlobalEnv));This is not safe. The call made by Rf_lang2() needs to be protected here.> For Rcpp_fast_eval, you may still need to #define RCPP_USE_UNWIND_PROTECT > before including Rcpp.h; as I recall we were seeing too many side effects in > other packages when globally enabling it. Or you can play it safe and simply > use Rcpp_eval. > > This really was more of a rcpp-devel question. > > Dirk > > -- > http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel
> Shield<SEXP> res(Rcpp_fast_eval(Rf_lang2(asEnvironmentSym, x), R_GlobalEnv));The call should be protected before evaluation though. So more like: Shield<SEXP> call(Rf_lang2(asEnvironmentSym, x)); return Rcpp_fast_eval(call, R_GlobalEnv); Best, Lionel On 3/23/20, Dirk Eddelbuettel <edd at debian.org> wrote:> > > On 23 March 2020 at 17:07, Ben Bolker wrote: > | Or is there a way I can use Shield() since this an Rcpp-based project > | anyway? > > Yes you can, and I would recommend it. > > Example from Rcpp itself, file Environment.h: > > Shield<SEXP> res(Rcpp_fast_eval(Rf_lang2(asEnvironmentSym, x), > R_GlobalEnv)); > > For Rcpp_fast_eval, you may still need to #define RCPP_USE_UNWIND_PROTECT > before including Rcpp.h; as I recall we were seeing too many side effects > in > other packages when globally enabling it. Or you can play it safe and > simply > use Rcpp_eval. > > This really was more of a rcpp-devel question. > > Dirk > > -- > http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >