hybrid evaluator did not use correct environment. closes #403

tidyverse · Apr 24, 2014 · 89fa6f9 · 89fa6f9
1 parent 26a7fab
commit 89fa6f9
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 2 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -141,6 +141,8 @@ dplyr 0.2 adds three new verbs:
 
 * `group_by` only creates one group for NA (#401). 
 
+* Hybrid evaluator did not evaluate expression in correct environment (#403). 
+
 # dplyr 0.1.3
 
 ## Bug fixes

diff --git a/inst/include/dplyr/Result/CallProxy.h b/inst/include/dplyr/Result/CallProxy.h
@@ -37,6 +37,7 @@ namespace dplyr {
                 for( int i=0; i<n; i++){
                     proxies[i].set( subsets[proxies[i].symbol] ) ;     
                 }
+
                 Shield<SEXP> res( call.eval(env) ) ;
                 return res ;
             } else if( TYPEOF(call) == SYMSXP) {

diff --git a/inst/include/dplyr/Result/GroupedHybridCall.h b/inst/include/dplyr/Result/GroupedHybridCall.h
@@ -15,7 +15,7 @@ namespace dplyr {
         SEXP eval(){
             if( TYPEOF(call) == LANGSXP ){
                 substitute(call) ;
-                return Rf_eval( call, R_GlobalEnv ) ;
+                return Rf_eval( call, env ) ;
             } else if(TYPEOF(call) == SYMSXP) {
                 if(subsets.count(call)){
                     return subsets.get(call, indices) ;    

diff --git a/inst/include/dplyr/Result/HybridCall.h b/inst/include/dplyr/Result/HybridCall.h
@@ -14,7 +14,7 @@ namespace dplyr {
         SEXP eval(){
             if( TYPEOF(call) == LANGSXP ){
                 substitute(call) ;
-                return Rf_eval( call, R_GlobalEnv ) ;
+                return Rf_eval( call, env ) ;
             } else if(TYPEOF(call) == SYMSXP) {
                 if(subsets.count(call)){
                     return subsets.get_variable(call) ;    

diff --git a/tests/testthat/test-mutate.r b/tests/testthat/test-mutate.r
@@ -201,3 +201,12 @@ test_that("mutate errors when results are not compatible accross groups (#299)",
 test_that("assignments are forbidden (#315)", {
    expect_error(mutate(mtcars, cyl2 = { x <- cyl^2; -x } ))
 })
+
+test_that("hybrid evaluator uses correct environment (#403)", {
+  func1 <- function() {
+    func2 <- function(x) floor(x)
+    mutate(mtcars, xx = func2(mpg / sum(mpg)))
+  }
+  res <- func1()
+  expect_equal(res$xx, rep(0,nrow(res)) )
+})