AttributeProxy::set() #947
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM! |
romainfrancois
commented
Mar 13, 2019
| } else { | ||
| /* use the slower and more flexible version (callback to R) */ | ||
| SEXP namesSym = Rf_install( "names<-" ); | ||
| Shield<SEXP> new_vec(Rcpp_fast_eval(Rf_lang3(namesSym, parent, x), R_GlobalEnv)); | ||
| Shield<SEXP> call(Rf_lang3(namesSym, parent, x)); |
There was a problem hiding this comment.
Was unsafe to not protect the result of Rf_lang3
romainfrancois
commented
Mar 13, 2019
| /* check if we can use a fast version */ | ||
| if( TYPEOF(x) == STRSXP && parent.size() == Rf_length(x) ){ | ||
| SEXP y = parent.get__() ; | ||
| Rf_setAttrib( y, R_NamesSymbol, x ) ; | ||
| Rf_namesgets(parent, x); |
There was a problem hiding this comment.
That's what Rf_namesgets is for.
romainfrancois
added a commit
to tidyverse/dplyr
that referenced
this pull request
Mar 14, 2019
romainfrancois
added a commit
to tidyverse/dplyr
that referenced
this pull request
Mar 14, 2019
romainfrancois
added a commit
to tidyverse/dplyr
that referenced
this pull request
Mar 15, 2019
romainfrancois
added a commit
to tidyverse/dplyr
that referenced
this pull request
Mar 15, 2019
romainfrancois
added a commit
to tidyverse/dplyr
that referenced
this pull request
Mar 15, 2019
* check_slice_result() no longer allocate, but protect anyway. rchk. * protect result of quo.env(). rchk * PROTECT quosure.expr(), as validate_unquoted_value() might alllocate * some more rchk protection * simplify get_date_classes() and get_time_classes() * need (SEXP) * some more rchk * protect results of vec_names_or_empty() * SymbolVector::init() protects its argument. rchk * more protection in DataMask::setup() * more rchk * using setAttrib() / getAttrib() instead of Rcpp's attr() to skip 💵 for Rf_install() * more rchk * more rchk. * more rchk * more rchk * simplify make_lambda_quosure * get_uniques() is never used * protect column_names. rchk * skip copies * protect result of wrap before the List ctor. * using r_match() instead of SymbolVector.match_in_table() * protect Rf_getAttrib(x, symbols::names) * more rchk * using Rcpp::export(rng = false). closes #4278 * rchk says the dtor of N_Distinct might allocate * scope the shared_ptr<> for rchk * protect from ~Gatherer() dtor. rchk * Cannot dirtectly send the result of match_in_table() in IntegerVector because of the unsafe ctor. rchk * trying RcppCore/Rcpp#949 to see if it helps quiet some rchk 😱 * - Remote * rm unused nprot * Shield<> before CharacterVector ctor, until RcppCore/Rcpp#947 gets merged in Rcpp. * Revert "- Remote" This reverts commit 72bb9aa. * SymbolMap(DataFrame) ctor to avoid rchk issue with the SymbolMap(CharacterVector) ctor * Forbid copy construction of GroupedDataFrame and SymbolMap * limit copies of SymbolVector * 🏃 a branch that incorporates changes from both Rcpp PRs: RcppCore/Rcpp#947 RcppCore/Rcpp#949 * use CRAN Rcpp
eddelbuettel
added a commit
that referenced
this pull request
Mar 16, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #946