Change to use SHALLOW_DUPLICATE_ATTRIB() since that's API since R3.3.0 (

#6264)

* More conservative PROTECT() around switch to API usage in asS4

* Use SHALLOW_DUPLICATE_ATTRIB

src/assign.c

@@ -151,10 +151,7 @@ static SEXP shallow(SEXP dt, SEXP cols, R_len_t n)
   // where n is set to truelength (i.e. a shallow copy only with no size change)
   int protecti=0;
   SEXP newdt = PROTECT(allocVector(VECSXP, n)); protecti++;   // to do, use growVector here?
-  SET_ATTRIB(newdt, shallow_duplicate(ATTRIB(dt)));
-  SET_OBJECT(newdt, OBJECT(dt));
-  if (isS4(dt)) newdt = asS4(newdt, TRUE, 1);  // To support S4 objects that include data.table
-  //SHALLOW_DUPLICATE_ATTRIB(newdt, dt);  // SHALLOW_DUPLICATE_ATTRIB would be a bit neater but is only available from R 3.3.0
+  SHALLOW_DUPLICATE_ATTRIB(newdt, dt);
 
   // TO DO: keepattr() would be faster, but can't because shallow isn't merely a shallow copy. It
   //        also increases truelength. Perhaps make that distinction, then, and split out, but marked

src/dogroups.c

@@ -482,8 +482,13 @@ SEXP keepattr(SEXP to, SEXP from)
   // Same as R_copyDFattr in src/main/attrib.c, but that seems not exposed in R's api
   // Only difference is that we reverse from and to in the prototype, for easier calling above
   SET_ATTRIB(to, ATTRIB(from));
-  if (isS4(from)) to = asS4(to, TRUE, 1);
-  SET_OBJECT(to, OBJECT(from));
+  if (isS4(from)) {
+    to = PROTECT(asS4(to, TRUE, 1));
+    SET_OBJECT(to, OBJECT(from));
+    UNPROTECT(1);
+  } else {
+    SET_OBJECT(to, OBJECT(from));
+  }
   return to;
 }