-
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
Conversation
Why not just wrap the foreign function in a safe, |
@cramertj |
@petrochenkov Should we also exclude functions consuming pointer types? Rust can generate arbitrary pointers in safe code and it would probably be a bad idea to pass them to a C function (unless it does not actually dereference the pointer, which seems a pretty niche case). |
} | ||
|
||
#[repr(C)] | ||
union LARGE_INTEGER { |
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.
[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 comment
The 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 unsafe
before the extern
, similar to unsafe impl
?
I'm not sure, but isn't one danger that E.g. take a function taking a mutable raw pointer. Right now this is unsafe by default. If you mark that function This would make it not easier, but harder to use FFI as the feature would be a common pitfall for users unfamiliar to it. You could try to forbid any raw pointers, but what if its hidden somewhere in a nested struct. So I don't think this feature is feasible. Also, while I agree to better tooling to generate FFI bindings to foreign languages, it should not come at a cost to make it easier to write unsafe code. Unsafe code should be hard to write so that it is avoided as often as possible. |
Linking @nikomatsakis's and @wycats's thoughts regarding this issue. |
Regarding raw pointers. Yes, this is a valid issue - people have to understand what they do when they write As a side note, foreign functions typically expect valid pointers, so they can usually use |
It's already simple enough to write an |
I agree to the part of the RFC concerning statics, if mutating that global static is still unsafe. |
I guess my feeling is that trivial functions are boilerplate, but not unnecessary boilerplate. Moving from unsafe to safe is an important thing to do in a program and should not be done without pause, even where it seems obvious. IMO this RFC makes that too easy to do. I also feel like the specific mechanism proposed adds complexity in a subtle way - it turns a 'marker' keyword into something more active. From the perspective of someone exploring the language it adds the following kinds of complexity:
And so forth. |
I'd like to speak up in favor of this RFC -- or at least the intuition behind it. Specifically, while I am not a fan of the Basically the best you can do is to have a function which reads the constant: extern {
static SENTINEL: SomeType;
}
fn sentinel() -> SomeType {
unsafe {
SENTINEL
}
} This is not great, both from an ergonomics POV (extra parens, etc) and because it's less efficient. Plus you may want two such functions, one for taking the address of the sentinel, etc. I am not sure how I want to solve this problem, though. Just letting us declare extern functions as "not unsafe" might not be the right way to go. For the case of statics, one could imagine a more targeted fix, e.g. a way to declare those statics as being constant. Another option might be improving unsafe itself -- I've been toying with the idea of unsafe functions that let you give more information about why the function is unsafe. Perhaps if we had this expanded form, you could also use that to declare that the function is safe. I don't have a very firm idea here though. Mostly I was thinking that if one were giving, as part of an unsafe, a list of conditions that ought to be checked, it might be possible for that list to be declared as explicitly empty or something like that. I'll note one thing about function wrappers: they are indeed mostly equivalent, but not entirely. For example, when converting them to an address in memory, you will get a different result. And the compiler has to inline them and hence they generally incur more compilation time. Those problems alone doesn't seem like they merit the solution in this RFC (as opposed to statics, which have no compelling alternative), but it's worth mentioning. |
We talked about this RFC extensively in the @rust-lang/lang meeting. I think there was a general consensus that:
I think the direction that I personally would prefer is to work on building a tool, either via build.rs or via procedural macros, that makes it really easy to create wrappers, shims, and in general the "glue" needed to connect to C (and eventually C++) code. I elaborated on these ideas in a comment on the roadmap thread, which I will excerpt here (the original comment goes into more detail):
I think this tool will require a certain amount of language extensions to work. For example, I don't see how it could generate code that handles constant statics, as I mentioned, and I think some people may want the ability to have a "safe fn pointer" to C code that is known to be safe. But having the tool in place should let us more easily distinguish things that can be done via wrapping and what are the "core requirements" (I would expect the tool to make it easy to wrap unsafe C functions, at least as a starting point, with a safe wrapper.) Eventually I would want to integrate the tool more into rustc; I think for good C++ integration we will want to be able to provide feedback, e.g. to obtain the list of specializations we need for a template, or to generate shims that will handle C++ overloading. But that's far in the future anyway. Sadly, I think a tool like this will need some champion to really get it going. @petrochenkov perhaps you are this person! :) |
I'll close this then to save everyone's time.
I don't think I'm ready to take such a responsibility now. |
Foreign functions and statics are unsafe by default, provide a way to mark them as safe.
Rendered