-
-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
safer Function<>::operator(), Rcpp_list*(), Rcpp_lang*() #1014
safer Function<>::operator(), Rcpp_list*(), Rcpp_lang*() #1014
Conversation
} | ||
|
||
*/ | ||
template <typename T1> | ||
SEXP operator()(const T1& t1) const { | ||
return Rcpp_fast_eval(Rcpp_lcons(Storage::get__(), pairlist(t1)), R_GlobalEnv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the result of pairlist()
was not protected, and Rcpp_lcons()
allocates, so it might gc() it.
the result of Rcpp_lcons()
is not protected, so it might be claimed when Rcpp_fast_eval()
allocates ...
all changes in that file are the same.
inline SEXP Rcpp_list7( SEXP x0, SEXP x1, SEXP x2, SEXP x3, SEXP x4, SEXP x5, SEXP x6 ) | ||
{ | ||
Shield<SEXP> out( Rf_cons(x0, Rcpp_list6(x1, x2, x3, x4, x5, x6)) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rf_cons()
allocates so it might claim x0 and the result of Rcpp_list6()
.
@@ -111,6 +111,14 @@ namespace Rcpp{ | |||
Shield<SEXP> x( Rf_findFun( nameSym, env ) ) ; | |||
Storage::set__(x) ; | |||
} | |||
|
|||
SEXP invoke(SEXP args_, SEXP env) const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a private support function to make operator()
easier to write.
Thanks for the PR. Next you consider a new PR, please also consider taking another look at the file Contributing. We really prefer issue tickets before PRs. Thank you. |
Sure. I opened #915. Edit by DE Removed uncalled-for tone. |
LGTM! If I understand correctly, this basically mirrors what R itself does when constructing pairlists as well. Do you think it would be worthwhile to implement a variadic C++11 version that avoids code bloat for these as well? |
Definitely. |
930827d
to
3af5556
Compare
C.f. RcppCore/rcpp-logs@0e28b87 Rebased to |
comments in context.