r-lib / cpp11

cpp11 helps you to interact with R objects using C++ code.
https://cpp11.r-lib.org/
Other
201 stars 48 forks source link

Investigate and remove `Rf_findVarInFrame3` usage in `environment` #367

Closed DavisVaughan closed 2 months ago

DavisVaughan commented 3 months ago

https://github.com/r-lib/cpp11/blob/1c9dbb65cc9279a404f035f2fca990b338c9f602/inst/include/cpp11/environment.hpp#L37-L51

DavisVaughan commented 3 months ago

Likely going to use R_existsVarInFrame() in exists() on R >=4.2.0


And then likely going to use the new R_getVarEx() on >=R-devel, although it has different semantics than Rf_findVarInFrame3(env, sym, TRUE) in edge cases, i.e.:

I don't think there is anything we can do about this unless we get an iterator API for environments that doesn't add special behavior like this

DavisVaughan commented 3 months ago

Lionel and I decided that for the proxy SEXP conversion method, we should use the new R_getVar() function (not the extended version), which would:

https://github.com/wch/r-source/blob/0986e80068addc5c884d06580412ff2331436814/src/main/envir.c#L2305

And then for the fallback for older R versions we call Rf_findVarInFrame3() but then add 3 checks to reimplement the 3 bullets from above, so the behavior is consistent on all R versions.

After the enum / iterator API for environments is released, people will have more options if they want to manually implement more advanced behaviors than this "safe" way.