Skip to content

Lang: Stabilize usage of rustc_nonnull_optimization_guaranteed on -1 #97122

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

Closed
joshtriplett opened this issue May 17, 2022 · 6 comments
Closed
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

I/O Safety (#87074) uses types which are repr(transparent) wrappers around file descriptors or sockets, and which have a niche at -1 which is an invalid fd/socket. Those types use rustc_nonnull_optimization_guaranteed, which together with the niche, allows using things like Option<OwnedFd> in FFI.

(The attribute remains unstable, and is misnamed at this point since it shouldn't say "nonnull"; it should be something like rustc_niche_optimization_guaranteed, but that shouldn't block this issue.)

Before stabilizing those types, we'd like to confirm T-lang consensus that we have no objections to exposing a type with a guaranteed non-zero niche (in this case, a guaranteed niche at -1). This commits us to continuing to provide some mechanism (even if perma-unstable) for declaring non-zero niches and allowing their use in FFI.

@joshtriplett joshtriplett added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label May 17, 2022
@joshtriplett
Copy link
Member Author

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 17, 2022

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 17, 2022
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 24, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented May 24, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett
Copy link
Member Author

@sunfishcode Should OwnedSocket have the same optimization applied?

@sunfishcode
Copy link
Member

@joshtriplett Yes, it should, and it does.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 3, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Jun 3, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants