Skip to content
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

runtime: deprecate SetFinalizer #70425

Open
cagedmantis opened this issue Nov 18, 2024 · 4 comments
Open

runtime: deprecate SetFinalizer #70425

cagedmantis opened this issue Nov 18, 2024 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cagedmantis
Copy link
Contributor

cagedmantis commented Nov 18, 2024

The original intent of proposal #67535 included the deprecation of SetFinalizer. While the proposal is approved and the implementation has landed in the tree, we feel less comfortable with deprecating SetFinalizer in the same release as the addition of AddCleanup. Notibly, the lack of a trivial migration from SetFinalizer to AddCleanup was an issue that was mentioned. This issue intends to track the deprecation of SetFinalizer in a future release of Go and the discussion surrounding it.

@mknyszek @cagedmantis @ianlancetaylor

@cagedmantis cagedmantis added NeedsFix The path to resolution is known, but the work has not been done. compiler/runtime Issues related to the Go compiler and/or runtime. labels Nov 18, 2024
@cagedmantis cagedmantis added this to the Go1.25 milestone Nov 18, 2024
@gabyhelp
Copy link

Related Issues

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@mknyszek
Copy link
Contributor

mknyszek commented Nov 18, 2024

To be clear, it may end up that we don't do this at all. I agree with @ianlancetaylor's point that the lack of a trivial migration path makes it hard to justify yammering at people about in IDE pop-ups. It's unfortunate, but given that SetFinalizer's problems were due to the core API, it might not be worth doing a full deprecation.

@dmitshur
Copy link
Contributor

dmitshur commented Nov 19, 2024

Noting here that https://go.dev/wiki/Deprecated is the place where it is documented that:

If function F1 is being replaced by function F2 and the first release in which F2 is available is Go 1.N, then an official deprecation notice for F1 should not be added until Go 1.N+1. This ensures that Go developers only see F1 as deprecated when all supported Go versions include F2 and they can easily switch.

Also that:

Marking an API feature deprecated can create work and decisions for millions of Go developers using the feature. Deprecating an API feature is an API change that must be discussed using the proposal process.

Since deprecation was a part of the accepted proposal #67535 it seems that's done, but if you think this needs more discussion, you might want to take advantage of that process again. Otherwise you can just refer to that existing accepted proposal.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/634318 mentions this issue: runtime: improve AddCleanup documentation

gopherbot pushed a commit that referenced this issue Dec 7, 2024
Steer people from SetFinalizer to AddCleanup. Address some of the
*non*-constraints on AddCleanup. Add some of the subtlety from the
SetFinalizer documentation to the AddCleanup documentation.

Updates #67535.
Updates #70425.

Change-Id: I8d13b756ca866051b8a5c19327fd5a76f5e0f3d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/634318
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Austin Clements <austin@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

5 participants