Skip to content

Conversation

@DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Oct 22, 2025

Quite a few R 4.5.0 helpers to make some more progress here. Commits are self contained.

Before / after

   File ‘rlang/libs/rlang.so’:
     Found non-API calls to R: ‘ENCLOS’, ‘OBJECT’, ‘PRENV’, ‘PRVALUE’,
       ‘R_PromiseExpr’, ‘Rf_allocSExp’, ‘Rf_findVarInFrame3’, ‘SETLENGTH’,
       ‘SET_BODY’, ‘SET_CLOENV’, ‘SET_ENCLOS’, ‘SET_FORMALS’,
       ‘SET_GROWABLE_BIT’, ‘SET_TRUELENGTH’, ‘SET_TYPEOF’, ‘XTRUELENGTH’


   File ‘rlang/libs/rlang.so’:
     Found non-API calls to R: ‘PRENV’, ‘PRVALUE’, ‘R_PromiseExpr’,
       ‘Rf_findVarInFrame3’, ‘SETLENGTH’, ‘SET_BODY’, ‘SET_CLOENV’,
       ‘SET_ENCLOS’, ‘SET_GROWABLE_BIT’, ‘SET_TRUELENGTH’

Fully dropped

  • ENCLOS
  • OBJECT
  • Rf_allocSExp
  • SET_FORMALS

Not shown but also dropped for vctrs via this

  • BODY
  • CLOENV

Remaining

  • PRENV, PRVALUE, R_PromiseExpr (i.e. PREXPR) all need delayed expression utils from our patch
  • Rf_findVarInFrame3 very complicated one tied up with the new R_GetVar and "missing" semantics
  • SETLENGTH, SET_GROWABLE_BIT, SET_TRUELENGTH for RESIZE(), no known plan here yet
  • SET_BODY in fn_zap_srcref(), could probably make a new function instead? We already do a clone of the function itself, so probably just as expensive. (Remove SET_BODY() usage #1845)
  • SET_CLOENV tied up in on_exit_restore_lexical_env() but it looks like you can just make a new function here too? (Remove SET_CLOENV() usage #1846)
  • SET_ENCLOS part of r_env_poke_parent() used in lots of tidy-eval things (Remove SET_ENCLOS() usage #1847)

May make follow up PRs for SET_BODY and SET_CLOENV, but they are more complicated so should be their own PRs

Comment on lines +18 to 23
// Identical to `R_BytecodeExpr(R_ClosureBody(fn))`, which we always want
// since it matches the R level `body()`
static inline
void r_fn_poke_body(r_obj* fn, r_obj* body) {
SET_BODY(fn, body);
r_obj* r_fn_body(r_obj* fn) {
return R_ClosureExpr(fn);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here R_ClosureExpr() is the same as BODY_EXPR(), which I think only still exists because of us

https://github.com/wch/r-source/blob/4241940e50d0d60cd459cf162ddb5e8ecd0307b3/src/include/Rinternals.h#L1285

It's worth noting that R_ClosureBody() also exists which directly calls BODY(), but the previous BODY_EXPR() and R_ClosureExpr() also handle bytecode stuff by doing R_BytecodeExpr(R_ClosureBody(fn)), which I think we want, so I have not exposed the new R_ClosureBody() at all.

@DavisVaughan DavisVaughan requested a review from lionel- October 22, 2025 16:13
This was referenced Oct 22, 2025
@DavisVaughan DavisVaughan merged commit f5b6476 into main Oct 23, 2025
13 checks passed
@DavisVaughan DavisVaughan deleted the feature/rlang-c-api branch October 23, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants