Skip to content
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

use exported R API when removing variables from environment #990

Closed
kevinushey opened this issue Sep 3, 2019 · 2 comments
Closed

use exported R API when removing variables from environment #990

kevinushey opened this issue Sep 3, 2019 · 2 comments

Comments

@kevinushey
Copy link
Contributor

From a recent R-devel post:

Thanks for spotting this pattern. Using Rf_defineVar() this way to
delete a variable from package code is wrong because WRE does not
describe such behavior. That behavior has not been intended by the
implementation and as you observe, the variable will not be properly
deleted.

I found another pattern in another package (Rcpp, Rcpp11) which calls
directly the .Internal() function that deletes a variable, that is also
wrong, one should only use exported wrappers. The only legitimate way to
delete a variable in already released versions of R from C is to use
eval() to call R's rm()/remove(), but we will consider adding a C
function to the API to do this directly.

And the code:

/* unless we want to copy all of do_remove,
we have to go back to R to do this operation */
SEXP internalSym = Rf_install( ".Internal" );
SEXP removeSym = Rf_install( "remove" );
Shield<SEXP> call( Rf_lang2(internalSym,
Rf_lang4(removeSym, Rf_mkString(name.c_str()), Storage::get__(), Rf_ScalarLogical( FALSE ))
) );
Rcpp_fast_eval( call, R_GlobalEnv ) ;

We're already calling back to R; might as well use the official function base::rm.

@eddelbuettel
Copy link
Member

Thanks. I follow r-devel pretty closely but missed this. [ Turns out I was behind on that folder. Busy day. ] Well and we know how the OP "really" feels about Rcpp, don't we ;-)

@github-actions
Copy link

This issue is stale (365 days without activity) and will be closed in 31 days unless new activity is seen. Please feel free to re-open it is still a concern, possibly with additional data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants