Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork219
safer Function<>::operator(), Rcpp_list*(), Rcpp_lang*()#1014
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| */ | ||
| template<typename T1> | ||
| SEXPoperator()(const T1& t1)const { | ||
| returnRcpp_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 ofpairlist() was not protected, andRcpp_lcons() allocates, so it might gc() it.
the result ofRcpp_lcons() is not protected, so it might be claimed whenRcpp_fast_eval() allocates ...
all changes in that file are the same.
| inline SEXPRcpp_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 ofRcpp_list6().
| Storage::set__(x) ; | ||
| } | ||
| SEXPinvoke(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 makeoperator() easier to write.
eddelbuettel commentedNov 8, 2019
Thanks for the PR. Next you consider a new PR, please also consider taking another look at the fileContributing. We really prefer issue tickets before PRs. Thank you. |
romainfrancois commentedNov 8, 2019 • edited by eddelbuettel
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by eddelbuettel
Uh oh!
There was an error while loading.Please reload this page.
Sure. I opened#915. Edit by DE Removed uncalled-for tone. |
kevinushey commentedNov 8, 2019
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? |
romainfrancois commentedNov 8, 2019
Definitely. |
930827d to3af5556Compareeddelbuettel commentedNov 10, 2019
C.f.RcppCore/rcpp-logs@0e28b87 Rebased to |
Uh oh!
There was an error while loading.Please reload this page.
comments in context.