-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Reverting default unsafety #1901
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
- Feature Name: not_unsafe | ||
- Start Date: 2017-02-15 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Provide ability to mark unsafe-by-default entities, like foreign items, as safe. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Foreign functions, foreign statics and union fields are defined as unsafe by | ||
default. | ||
|
||
Often it's statically known that calling a certain foreign function or accessing | ||
a static variable is safe, either because the other side of FFI is under control | ||
of the same person, or the behavior of that function/static is well documented | ||
as being safe. | ||
|
||
A foreign function may, for example, be a pure math function from a C library. | ||
Or it may be a safe Rust intrinsic, which are defined as foreign functions as | ||
well. | ||
A | ||
[comment](https://github.com/rust-lang/rust/issues/36247#issuecomment-247903943) | ||
on tracking issue for `safe_extern_statics` compatibility lint provides a use | ||
case for safe foreign statics - modeling MMIO registers. | ||
|
||
Currently there are two ways to communicate this statically known safety to the | ||
compiler. | ||
|
||
The first way is just to surround calls/accesses to unsafe items with | ||
`unsafe` blocks on each use, asserting that those calls/accesses are indeed | ||
safe. This is verbose and creates too many superfluous `unsafe` blocks where | ||
only one `unsafe` would suffice, reducing the value of others "truly unsafe" | ||
blocks. | ||
|
||
Alternatively, a safe wrapper function can be implemented and call/access | ||
the unsafe item internally using `unsafe` block. | ||
This way `unsafe` is shifted from each point of use to a single location, | ||
similarly to how unsafe trait like `Send` is implemented for a type once and | ||
then asserted in many locations. | ||
|
||
This RFC proposes to shift the `unsafe` further on the foreign item itself | ||
removing the need in wrapper function boilerplate that often serves no other | ||
purpose than removing unsafety. | ||
|
||
The proposal fits into 2017 roadmap as an improvement to ergonomics and | ||
integration with other languages. | ||
|
||
# Detailed design | ||
[design]: #detailed-design | ||
|
||
Functions and statics in foreign modules as well as union fields can be | ||
prepended with `!unsafe` and safety checker will treat them as safe. Example: | ||
```rust | ||
extern "C" { | ||
!unsafe fn f(); | ||
!unsafe static S: u8; | ||
} | ||
|
||
#[repr(C)] | ||
union LARGE_INTEGER { | ||
!unsafe qword: u64, | ||
!unsafe dwords: (u32, u32), | ||
} | ||
``` | ||
|
||
Mutable statics cannot be declard as `!unsafe`. | ||
Variadic foreign functions (e.g. `printf`) cannot be declared as `!unsafe`. | ||
|
||
# How We Teach This | ||
[how-we-teach-this]: #how-we-teach-this | ||
|
||
With a paragraph of text and an example. | ||
The syntax is intuitive and mirrors existing `unsafe` functions. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have different opinions of intuitive ;). Double negatives are always confusing to me. Maybe just add the |
||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
None known. | ||
|
||
# Alternatives | ||
[alternatives]: #alternatives | ||
|
||
Use contextual keyword `safe` instead of `!unsafe`. | ||
This goes against the general rule "something bad happens -> search for | ||
`unsafe`". Now you'll have to search for `safe` as well. | ||
|
||
Postpone `!unsafe` on union fields and combine it in one proposal with `unsafe` | ||
on struct fields. | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
None. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd much rather change the (unstable) unions to require
unsafe
before them (always). Then in the future, we can add safe unions (like yours), which are checked by the compiler for safety.