Prefer Symbol to String (#2185)

* input() works with symbols

* has_group() works with symbols

* use Symbol more

inst/include/dplyr/Gatherer.h

@@ -302,7 +302,7 @@ namespace dplyr {
   }
 
   template <typename Data, typename Subsets>
-  inline Gatherer* gatherer(GroupedCallProxy<Data,Subsets>& proxy, const Data& gdf, SEXP name) {
+  inline Gatherer* gatherer(GroupedCallProxy<Data,Subsets>& proxy, const Data& gdf, Symbol name) {
     typename Data::group_iterator git = gdf.group_begin();
     typename Data::slicing_index indices = *git;
     RObject first(proxy.get(indices));
@@ -341,7 +341,7 @@ namespace dplyr {
       break;
     }
 
-    check_supported_type(first, name);
+    check_supported_type(first, name.c_str());
     return 0;
   }
 

inst/include/dplyr/GroupedDataFrame.h

@@ -110,11 +110,10 @@ namespace dplyr {
       return biggest_group_size;
     }
 
-    inline bool has_group(SEXP g) const {
-      SEXP symb = Rf_installChar(g);
+    inline bool has_group(Symbol g) const {
       int n = symbols.size();
       for (int i=0; i<n; i++) {
-        if (symbols[i] == symb) return true;
+        if (symbols[i] == g) return true;
       }
       return false;
     }

inst/include/dplyr/Groups.h

@@ -6,9 +6,6 @@
 #include <dplyr/GroupedDataFrame.h>
 #include <dplyr/RowwiseDataFrame.h>
 
-void check_not_groups(const CharacterVector& result_names, const GroupedDataFrame& gdf);
-void check_not_groups(const CharacterVector& result_names, const RowwiseDataFrame& gdf);
-
 void check_not_groups(const LazyDots& dots, const GroupedDataFrame& gdf);
 void check_not_groups(const LazyDots& dots, const RowwiseDataFrame& gdf);
 

inst/include/dplyr/NamedListAccumulator.h

@@ -15,9 +15,9 @@ namespace dplyr {
 
     NamedListAccumulator() {}
 
-    inline void set(SEXP name, SEXP x) {
+    inline void set(Symbol name, SEXP x) {
       if (! Rcpp::traits::same_type<Data, RowwiseDataFrame>::value)
-        check_supported_type(x, name);
+        check_supported_type(x, name.c_str());
 
       SymbolMapIndex index = symbol_map.insert(name);
       if (index.origin == NEW) {

inst/include/dplyr/Result/GroupedCallProxy.h

@@ -67,8 +67,8 @@ namespace dplyr {
       if (TYPEOF(call) == LANGSXP) traverse_call(call);
     }
 
-    void input(Rcpp::String name, SEXP x) {
-      subsets.input(Rf_installChar(name.get_sexp()) , x);
+    void input(Symbol name, SEXP x) {
+      subsets.input(name, x);
     }
 
     inline int nsubsets() {

inst/include/dplyr/check_supported_type.h

@@ -12,7 +12,7 @@ namespace dplyr {
     DPLYR_VECSXP = VECSXP
   };
 
-  inline SupportedType check_supported_type(SEXP x, SEXP name = R_NilValue) {
+  inline SupportedType check_supported_type(SEXP x, std::string name = "") {
     switch (TYPEOF(x)) {
     case LGLSXP:
       return DPLYR_LGLSXP;
@@ -27,11 +27,11 @@ namespace dplyr {
     case VECSXP:
       return DPLYR_VECSXP;
     default:
-      if (name == R_NilValue) {
+      if (name == "") {
         stop("Unsupported type %s", type2name(x));
       }
       else {
-        stop("Unsupported type %s for column \"%s\"", type2name(x), CHAR(name));
+        stop("Unsupported type %s for column \"%s\"", type2name(x), name);
       }
 
       // Unreachable, can be removed with Rcpp > 0.12.5.2

inst/include/tools/LazyDots.h

@@ -5,7 +5,7 @@ namespace Rcpp {
 
   class Lazy {
   public:
-    Lazy(List data_, SEXP name__) :
+    Lazy(List data_, Symbol name__) :
       data(data_),
       name_(name__)
     {}
@@ -21,14 +21,13 @@ namespace Rcpp {
     inline SEXP env() const {
       return data[1];
     }
-    inline SEXP name() const {
+    inline Symbol name() const {
       return name_;
     }
 
   private:
-
     List data;
-    SEXP name_;
+    Symbol name_;
   };
 
   template <>
@@ -53,7 +52,9 @@ namespace Rcpp {
         if (!is<Lazy>(x)) {
           stop("corrupt lazy object");
         }
-        data.push_back(Lazy(x, names[i]));
+
+        Symbol name(names[i]);
+        data.push_back(Lazy(x, name));
       }
     }
 

src/mutate.cpp

@@ -36,17 +36,8 @@ SEXP structure_mutate(const NamedListAccumulator<Data>& accumulator, const DataF
   return res;
 }
 
-void check_not_groups(const CharacterVector& result_names, const RowwiseDataFrame& gdf) {}
 void check_not_groups(const LazyDots& dots, const RowwiseDataFrame& gdf) {}
 
-void check_not_groups(const CharacterVector& result_names, const GroupedDataFrame& gdf) {
-  int n = result_names.size();
-  for (int i=0; i<n; i++) {
-    if (gdf.has_group(result_names[i]))
-      stop("cannot modify grouping variable");
-  }
-}
-
 void check_not_groups(const LazyDots& dots, const GroupedDataFrame& gdf) {
   int n = dots.size();
   for (int i=0; i<n; i++) {
@@ -65,7 +56,7 @@ SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots) {
   if (nvars) {
     CharacterVector df_names = df.names();
     for (int i=0; i<nvars; i++) {
-      accumulator.set(df_names[i], df[i]);
+      accumulator.set(Symbol(df_names[i]), df[i]);
     }
   }
 
@@ -78,7 +69,7 @@ SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots) {
 
     Shield<SEXP> call_(lazy.expr());
     SEXP call = call_;
-    SEXP name = lazy.name();
+    Symbol name = lazy.name();
     Environment env = lazy.env();
     call_proxy.set_env(env);
 
@@ -101,7 +92,7 @@ SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots) {
       stop("cannot handle");
     }
 
-    check_supported_type(results[i], name);
+    check_supported_type(results[i], name.c_str());
 
     if (Rf_inherits(results[i], "POSIXlt")) {
       stop("`mutate` does not support `POSIXlt` results");
@@ -152,7 +143,7 @@ SEXP mutate_grouped(const DataFrame& df, const LazyDots& dots) {
   int ncolumns = df.size();
   CharacterVector column_names = df.names();
   for (int i=0; i<ncolumns; i++) {
-    accumulator.set(column_names[i], df[i]);
+    accumulator.set(Symbol(column_names[i]), df[i]);
   }
 
   LOG_VERBOSE << "processing " << nexpr << " variables";
@@ -165,7 +156,7 @@ SEXP mutate_grouped(const DataFrame& df, const LazyDots& dots) {
     Environment env = lazy.env();
     Shield<SEXP> call_(lazy.expr());
     SEXP call = call_;
-    SEXP name = lazy.name();
+    Symbol name = lazy.name();
     proxy.set_env(env);
 
     LOG_VERBOSE << "processing " << CharacterVector(name);
@@ -177,7 +168,7 @@ SEXP mutate_grouped(const DataFrame& df, const LazyDots& dots) {
         accumulator.set(name, variable);
       } else {
         SEXP v = env.find(CHAR(PRINTNAME(call)));
-        check_supported_type(v, name);
+        check_supported_type(v, name.c_str());
         if (Rf_isNull(v)) {
           stop("unknown variable: %s", CHAR(PRINTNAME(call)));
         } else if (Rf_length(v) == 1) {