FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Require R >=3.5.0 and remove `release_all()` by DavisVaughan · Pull Request #332 · r-lib/cpp11 · GitHub

/ cpp11 Public

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

Merged
DavisVaughan merged 4 commits into
r-lib:mainfrom
DavisVaughan:feature/remove-release-all
Aug 3, 2023
Merged

DavisVaughan merged 4 commits into
r-lib:mainfrom
DavisVaughan:feature/remove-release-all

Conversation

DavisVaughan commented Aug 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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 force-pushed the feature/remove-release-all branch from 1a02484 to 398ac94 Compare August 3, 2023 12:55
DavisVaughan merged commit 9bb8f71 into r-lib:main Aug 3, 2023
DavisVaughan deleted the feature/remove-release-all branch August 3, 2023 13:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL