Skip to content

Require R >=3.5.0 and remove release_all() #332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 3, 2023

Conversation

DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Aug 2, 2023

Tied to #331, where we switch from a global preserve list to 1 preserve list per compilation unit. release_all() was intended to support experts that wanted to manually release cpp11 protected objects on R < 3.5.0, but it no longer is applicable for a few reasons:

  • In the tidyverse we now require R >=3.6.0 in most places
  • Using 1 preserve list per compilation unit means it will now only release objects managed by that compilation unit's preserve list
  • We think the idea of potentially releasing objects you don't own can be dangerous (like if you accidentally release vroom ALTREP objects, which can persist past a .Call() boundary)

No one was using release_all() as far as we can tell.


I've also bumped the minimum R version to >=3.5.0, so we no longer need code that supports the "old times" before we had access to R_UnwindProtect(). To this end, I've removed CPP11_USE_PRESERVE_OBJECT as well.

I've kept some details in the internals vignette, because I think the discussion of the alternative approaches is still useful for historical context.

At this point in r-lib and the tidyverse we typically require R >= 3.6.0, so it is reasonable to finally require this and remove our suboptimal fallback paths.
Which no longer make sense if we have 1 preserve list per compilation unit and require R >=3.5.0.

It does not seem to be used by anyone.
@DavisVaughan DavisVaughan force-pushed the feature/remove-release-all branch from 1a02484 to 398ac94 Compare August 3, 2023 12:55
@DavisVaughan DavisVaughan merged commit 9bb8f71 into r-lib:main Aug 3, 2023
@DavisVaughan DavisVaughan deleted the feature/remove-release-all branch August 3, 2023 13:10
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