Remove {UN,}SET_S4_OBJECT usage (#6183)

* Remove {UN,}SET_S4_OBJECT usage

* handle asS4 from R side

* tweak NES

* switch to C API

* restore dogroups code using public API

* new test

* adjust NEWS

* IS_S4_OBJECT --> isS4

NEWS.md

@@ -110,6 +110,8 @@
 
 20. Removed a warning about the now totally-obsolete option `datatable.CJ.names`, as discussed in previous releases.
 
+21. Refactored some non-API calls to R macros for S4 objects (#6180)[https://github.com/Rdatatable/data.table/issues/6180]. There should be no user-visible change. Thanks to various R users & R core for pushing to have a clearer definition of "API" for R, and thanks @MichaelChirico for implementing here.
+
 ## TRANSLATIONS
 
 1. Fix a typo in a Mandarin translation of an error message that was hiding the actual error message, [#6172](https://github.com/Rdatatable/data.table/issues/6172). Thanks @trafficfan for the report and @MichaelChirico for the fix.

inst/tests/S4.Rraw

@@ -102,3 +102,10 @@ if (TZnotUTC) {
        fread("a,b,c\n2015-01-01,2015-01-02,2015-01-03 01:02:03", colClasses=c("Date",NA,NA)),
        ans, output=ans_print)
 }
+
+# S4 object in grouping output requiring growVector
+#   coverage test towards refactoring for #6180
+DT = data.table(a = rep(1:2, c(1, 100)))
+# Set the S4 bit on a simple object
+DT[, b := asS4(seq_len(.N))]
+test(6, DT[, b, by=a, verbose=TRUE][, isS4(b)], output="dogroups: growing")

src/assign.c

@@ -153,7 +153,7 @@ static SEXP shallow(SEXP dt, SEXP cols, R_len_t n)
   SEXP newdt = PROTECT(allocVector(VECSXP, n)); protecti++;   // to do, use growVector here?
   SET_ATTRIB(newdt, shallow_duplicate(ATTRIB(dt)));
   SET_OBJECT(newdt, OBJECT(dt));
-  IS_S4_OBJECT(dt) ? SET_S4_OBJECT(newdt) : UNSET_S4_OBJECT(newdt);  // To support S4 objects that include data.table
+  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
 
   // TO DO: keepattr() would be faster, but can't because shallow isn't merely a shallow copy. It

src/dogroups.c

@@ -482,7 +482,7 @@ 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));
-  IS_S4_OBJECT(from) ?  SET_S4_OBJECT(to) : UNSET_S4_OBJECT(to);
+  if (isS4(from)) to = asS4(to, TRUE, 1);
   SET_OBJECT(to, OBJECT(from));
   return to;
 }