Tweak coverage #1435
Tweak coverage #1435
Conversation
| // This is used internally instead of Rf_eval() to make evaluation safer | ||
| inline SEXP Rcpp_eval_impl(SEXP expr, SEXP env) { | ||
| return Rcpp_fast_eval(expr, env); | ||
| inline SEXP Rcpp_eval_impl(SEXP expr, SEXP env) { // #nocov |
There was a problem hiding this comment.
Shouldn't we drop this and use Rcpp_fast_eval directly in the remaining spots?
There was a problem hiding this comment.
Well spotted. And when you step back and look at that file you see this in a 'else' branch we no longer need (as we're always greater or equal R 3.5.0 now) and with it the whole UNWIND_PROTECT define can likely go away. I had been meaning to ping you about that. Cleanup for the next release cycle?
|
|
||
| /* check if we can use a fast version */ | ||
| if( TYPEOF(x) == STRSXP && parent.size() == Rf_length(x) ){ | ||
| if( TYPEOF(x) == STRSXP && parent.size() == Rf_length(x) ){ // #nocov start |
There was a problem hiding this comment.
Maybe we should design a test to check the other side of the branch instead?
There was a problem hiding this comment.
In theory, just about every instance of #nocov could deserve a test. Some of this areas have been without one for twelve or more years though. Adding tests is always the hope, in practice it does not happen all that much.
| try{ | ||
| index = parent.offset(name) ; | ||
| parent[ index ] = rhs ; | ||
| parent[ index ] = rhs ; // #nocov |
There was a problem hiding this comment.
Weird. Does this mean that we don't have any test that doesn't throw here?
There was a problem hiding this comment.
Maybe. That is indeed one of the weirder ones.
| /* We need to evaluate if it is a promise */ | ||
| if( TYPEOF(res) == PROMSXP){ | ||
| res = internal::Rcpp_eval_impl( res, env ) ; | ||
| res = internal::Rcpp_eval_impl( res, env ) ; // #nocov |
There was a problem hiding this comment.
Looks like this is important enough to test it?
There was a problem hiding this comment.
Sure. PRs always welcome. Or if you want to add a quick test now we can use this PR.
| typedef XPtr<class_Base> XP_Class ; | ||
| typedef Rcpp::XPtr<Rcpp::Module> XP ; | ||
| CppClass( SEXP x) : S4(x){}; | ||
| CppClass( SEXP x) : S4(x){}; // #nocov |
There was a problem hiding this comment.
Maybe we should test instantiation from SEXP?
There was a problem hiding this comment.
Same as last comment.
This PR updates #nocov tags for a number of R and C++ source files. Apart from one remaining 'function factory' function that confuses the coverage counting, this pushes the needle towards full coverage.
Checklist
R CMD checkstill passes all tests