Re: trimming down the native API
Thanks for your comments, see my answers inline…
On 18 Jan 2017, at 8:01, Karl Millar <kmillar@...> wrote:The problem with using different names is that you have to come up with new names ;-)
- I think the changes to Rinterface.h and Rembedded.h may causeAh, yes, CRAN packages aren’t representative for that part of the API.
I actually think that this part should be extended with enough API to allow RStudio to work without copying stuff from GNU R header files, but that’s another story ;-)
- From Rinternals.h, we might want to keep the following functions forI disagree on those - they expose details about the GC, so that they shouldn’t be part of the API needlessly.
Rf_installDDVALThis is just a cached shortcut for sprintf(“..%d”, i) and Rf_install, I don’t see a reason for this to be in the API.
R_RegisterFinalizerEx, R_MakeWeakRefC are referenced in 'Writing RIndeed, we should keep them if they are referenced in the docs.
However, AFAICS this whole concept of weak references seems to be used so little that there could be a separate discussion as to whether it’s necessary, and whether there’s enough usages to be confident that it works in the first place.
- I'd also recommend looking closely at the functions required by theThe packages in src/library can still access everything - they have a closer relationship anyway, e.g., they use the Defn.h header file which spills many internal details.
Other functions and variables from Rinternals.h that I think we shouldThis one may actually be unused, hard to look for because it’s such a common identifier.
VECTOR_PTRI fear that Rcpp relies heavily on this..?
MISSING / SET_MISSINGSET_MISSING is only used once in the sprint package, MISSING appear to be unused.
INTERNALAnother function difficult to look for because of a common identifier :-) seems like it’s only used in Rcpp.
HASHTAB / SET_HASHTABThese are used by a couple of packages, but it seems doable.
PRSEENRcpp is the only user here, that should be possible.
SET_PRVALUEThis one looks like an oversight, it should be removed.
R_InBCInterpreterAgreed, no usages anyway.
R_RestartTokenAgreed, no usages anyway.
R_dot_*Agreed, no usages anyway.
R_signal_*_errorAgreed, no usages anyway.
R_curErrorBufRcpp11 and Rhpc are the only users at the moment, that should be possible.
R_cycle_detectedAgreed, no usages anyway.
Seems like we missed a few opportunities in the initial go.
We’ll update the github PR accordingly.