4 ms·

> 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 f

> 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.

mlindner 4y ago
> 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. If a safe function has requirements to maintain soundness then it's either an unsafe function or an unsound improperly-labeled safe function. It's a contradiction in terms to label a function safe while also having unsoundness that can be created by misuse of the function. > 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. The inner function (assuming it's within the scope of the outer function) I would say is still "part of" the outer function. Inner functions are in-effect just more complex scoping within the outer function. They're not first-class citizens. As long as it is impossible for the inner function to ever escape it's scope then I would still consider this safe as long as the outer function can safely accept any input. Again, making sure that all unsafe blocks are properly commented. I would make it extremely clear by putting the unsafe {} as starting at the beginning of the function and ending at the end. However there's a slight problem here after thinking about this a bit more. It must be solely the responsibility of the outer function that the inner function is used correctly. It cannot rely on the implementation of any piece of code outside the inner function. The acid test here is whether it is possible that if the implementation of Option::map() were to ever change (while itself still being sound) whether it is possible that it could expose unsoundness without violating the FnOnce preconditions, i.e. without causing a compiler error. If a language (or library) version upgrades can expose unsoundness in your code, then your function is unsound. It's unclear from your example if that would be the case. In general though I would say this is a poorly written function however and this type of thing should be avoided. > But having an invariant held across multiple functions does not automatically make your code unsound. Again, to that I would disagree. > 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. I would need a more concrete example to look at. However the callbacks from external code example sounds especially fraught. > All it takes is the recognition that not every single function is its own standalone interface. I would go even further and say it is an invariant of the Rust language itself that any safe function will never expose unsoundness.

mlindner 4y ago

> 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. If a safe function has requirements to maintain soundness then it's either an unsafe function or an unsound improperly-labeled safe function. It's a contradiction in terms to label a function safe while also having unsoundness that can be created by misuse of the function. > 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. The inner function (assuming it's within the scope of the outer function) I would say is still "part of" the outer function. Inner functions are in-effect just more complex scoping within the outer function. They're not first-class citizens. As long as it is impossible for the inner function to ever escape it's scope then I would still consider this safe as long as the outer function can safely accept any input. Again, making sure that all unsafe blocks are properly commented. I would make it extremely clear by putting the unsafe {} as starting at the beginning of the function and ending at the end. However there's a slight problem here after thinking about this a bit more. It must be solely the responsibility of the outer function that the inner function is used correctly. It cannot rely on the implementation of any piece of code outside the inner function. The acid test here is whether it is possible that if the implementation of Option::map() were to ever change (while itself still being sound) whether it is possible that it could expose unsoundness without violating the FnOnce preconditions, i.e. without causing a compiler error. If a language (or library) version upgrades can expose unsoundness in your code, then your function is unsound. It's unclear from your example if that would be the case. In general though I would say this is a poorly written function however and this type of thing should be avoided. > But having an invariant held across multiple functions does not automatically make your code unsound. Again, to that I would disagree. > 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. I would need a more concrete example to look at. However the callbacks from external code example sounds especially fraught. > All it takes is the recognition that not every single function is its own standalone interface. I would go even further and say it is an invariant of the Rust language itself that any safe function will never expose unsoundness.