5 ms·

> But it is possible to have a private safe function that can only possibly be accessed from a small part of your code base. This would also be unsound. Multip

> But it is possible to have a private safe function that can only possibly be accessed from a small part of your code base. This would also be unsound. Multiple people develop software in a library. Any future developer on your library will automatically assume that any safe Rust function can be called with any parameters and not invoke undefined behavior. That is the default assumption of any Rust developer. If you have such code in your libraries, you should fix them, or mark them as clearly unsafe such that any caller of that function ensures they keep the invariants that are required of that unsafe code that is propagated upward. You're falling back to old ideas from C. > If you are careful in preserving this invariant This is C/C++ mentality. The entire point about Rust is that you do not need to be careful. It's guaranteed and checked by the compiler. Again, if you can invoke undefined behavior by calling ANY safe function with ANY form of input, then the function is unsound. This is simply how Rust assumptions work. > But this obviously gets more risky, the more code that has access to the function. I'll grant that it's not an uncommon pitfall to assume a crate-wide invariant somewhere that you later forget about. I suggest you re-learn how to program Rust, personally. You've picked up some really bad habits that don't line up with general consensus on how to code in Rust.

Nemo157 4y ago
Since unsoundness is a property of an interface, that private safe function would be unsound but could be used as part of the implementation details of a sound public API. It's not something I would do personally, and would be something I'd put down as a black mark against using a library as a dependency, but it's nowhere near as bad as doing the same thing on a public API.
- mlindner 4y ago
  Every single function call is using an interface. Your public API is not the only interface. It's an extremely bad habit to pick up as this ruins one of the main purposes of Rust, namely it's ability to allow collaboration and while not causing undefined behavior. Any sort of internal "don't do this or it'll cause undefined behavior" is just falling back to C/C++ land. Some developer later will modify that code later, maybe even the same developer and will forget about the invariants that need to be held if they're not documented in unsafety comments around unsafe code. Anything that relies on code review to catch unsoundness outside of unsafe code is automatically wrong.
LegionMammal978 4y ago
> This would also be unsound. Multiple people develop software in a library. Any future developer on your library will automatically assume that any safe Rust function can be called with any parameters and not invoke undefined behavior. That is the default assumption of any Rust developer. That's quite the broad generalization. Obviously, if you're at all sane, you're internally documenting your internal invariants, and keeping the section of code upholding them as small and tightly-packed as possible. Rust's unsafe keyword is simply a tool that lets us use interfaces without being fully familiar with their implementations. It's generally a great idea to encapsulate unsafe code into a sound internal interface whenever possible, but this doesn't necessarily require that every single function be its own standalone interface. What matters is that it's clear to the maintainers that the callers (which should best be nearby in the code) are bound by its requirements. > Again, if you can invoke undefined behavior by calling ANY safe function with ANY form of input, then the function is unsound. This is simply how Rust assumptions work. Suppose I have a function that internally creates an Option<T> in such a way that a certain invariant is upheld. I call Option::map() on it, passing it an inner function with an unsafe block assuming that the invariant is upheld. The inner function is called nowhere else. By necessity, this inner function is not marked unsafe, since Option::map() requires a safe FnOnce. Is there unsoundness in my code? I say not, since the inner function is not a standalone interface; it is an implementation detail inseparable from its outer function. There is absolutely no way to invoke my code as it stands so as to cause UB. > I suggest you re-learn how to program Rust, personally. You've picked up some really bad habits that don't line up with general consensus on how to code in Rust. I would prefer that you not make baseless allegations about my code. I mention that pitfall only from experience reviewing others' unsafe code. Again, if you're sane, you keep the parts of the code manually upholding your invariants as small as possible. Marking your internal functions unsafe and clearly documenting their preconditions is a perfectly fine way to go, and that's how I write my own code, being a sane person. But having an invariant held across multiple functions does not automatically make your code unsound. In certain situations (internal implementations of external traits, callbacks from external code), it is even necessary to have an unsafe precondition pass through the call of a function or closure marked safe. All it takes is the recognition that not every single function is its own standalone interface.
- 4y ago

Nemo157 4y ago

Since unsoundness is a property of an interface, that private safe function would be unsound but could be used as part of the implementation details of a sound public API. It's not something I would do personally, and would be something I'd put down as a black mark against using a library as a dependency, but it's nowhere near as bad as doing the same thing on a public API.

mlindner 4y ago
Every single function call is using an interface. Your public API is not the only interface. It's an extremely bad habit to pick up as this ruins one of the main purposes of Rust, namely it's ability to allow collaboration and while not causing undefined behavior. Any sort of internal "don't do this or it'll cause undefined behavior" is just falling back to C/C++ land. Some developer later will modify that code later, maybe even the same developer and will forget about the invariants that need to be held if they're not documented in unsafety comments around unsafe code. Anything that relies on code review to catch unsoundness outside of unsafe code is automatically wrong.

LegionMammal978 4y ago

> This would also be unsound. Multiple people develop software in a library. Any future developer on your library will automatically assume that any safe Rust function can be called with any parameters and not invoke undefined behavior. That is the default assumption of any Rust developer. That's quite the broad generalization. Obviously, if you're at all sane, you're internally documenting your internal invariants, and keeping the section of code upholding them as small and tightly-packed as possible. Rust's unsafe keyword is simply a tool that lets us use interfaces without being fully familiar with their implementations. It's generally a great idea to encapsulate unsafe code into a sound internal interface whenever possible, but this doesn't necessarily require that every single function be its own standalone interface. What matters is that it's clear to the maintainers that the callers (which should best be nearby in the code) are bound by its requirements. > Again, if you can invoke undefined behavior by calling ANY safe function with ANY form of input, then the function is unsound. This is simply how Rust assumptions work. Suppose I have a function that internally creates an Option<T> in such a way that a certain invariant is upheld. I call Option::map() on it, passing it an inner function with an unsafe block assuming that the invariant is upheld. The inner function is called nowhere else. By necessity, this inner function is not marked unsafe, since Option::map() requires a safe FnOnce. Is there unsoundness in my code? I say not, since the inner function is not a standalone interface; it is an implementation detail inseparable from its outer function. There is absolutely no way to invoke my code as it stands so as to cause UB. > I suggest you re-learn how to program Rust, personally. You've picked up some really bad habits that don't line up with general consensus on how to code in Rust. I would prefer that you not make baseless allegations about my code. I mention that pitfall only from experience reviewing others' unsafe code. Again, if you're sane, you keep the parts of the code manually upholding your invariants as small as possible. Marking your internal functions unsafe and clearly documenting their preconditions is a perfectly fine way to go, and that's how I write my own code, being a sane person. But having an invariant held across multiple functions does not automatically make your code unsound. In certain situations (internal implementations of external traits, callbacks from external code), it is even necessary to have an unsafe precondition pass through the call of a function or closure marked safe. All it takes is the recognition that not every single function is its own standalone interface.

4y ago