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
Random access APIs should be marked as unsafe #21
Comments
If the runtime borrow checks are sound, i.e. guaranteed to catch any incorrect access by safe code in any build configuration, these should not be marked unsafe, but could perhaps be carefully warned about in documentation. |
An API that crashes the application rather than corupting state may not need to be marked as Also, runtime borrow checking may not always be in effect. |
While it's not unsound to mark safe functions as
If so, the rest is irrelevant and the interface absolutely is and must be marked unsafe. |
Put another way, the runtime borrow checking changes how the bug is expressed (stack trace vs state corruption), but it does nothing to change whether or not the bug exists, its cause, or what the programmer must do to ensure it does not happen. The runtime checks should be viewed as a diagnostic aid, not as a real behavioural change. Their inclusion does not do enough to obviate the usage warning that The semantics of |
I appreciate your point here, and I agree that the programmer should be aware of the requirements of the API and taking care not to run afoul of them, no matter what is said about the precise consequences of misuse. That said, bugs in safe vs. unsafe versions of an API may have the same cause and solution, but they do not have the same effects. This is the critical distinction that |
That the application crashes the process immediately before the CPU was about to write into a memory location that it was erronously programmed to perform due to a memory-safety bug does nothing to impact the nature of the problem. Whether or not that runtime check is performed has no impact on how the programmer must consider the code. The effects are not what matter - From the nomicon:
|
What's the point of this multiple-borrow query system? I think even in specs you get the same semantics as indexing into std containers (that is one mutable borrow at once), and in most cases borrow issues can be overcomed by splitting components or deferring write. |
To reiterate, it's not about the nature of a downstream programmer's bug, but rather describing the scope of possible outcomes of misuse. This is important because the consequences of undefined behavior can be much more severe than suddenly killing the process.
This argument only makes sense the hypothetical programmer does not need to account for the possibility of bugs arising in their code despite their best efforts. Bugs do happen, and therefore unsafe code needs to be treated differently by a programmer (for example, by subjecting it to additional review or adding
The nomicon isn't referring to completely arbitrary logical contracts there. Keep reading:
|
I'd like to bring up an alternative approach here: It would be ideal if the user can opt-in to the granularity (component type vs chunk vs entity) of the runtime borrow checking as well as the safety guarantees the API provides. This would allow for an entirely safe default, more granular safe options, and unsafe alternatives for access if performance is important for the caller. |
Fixed in #27. |
Random access APIs, such as
World.get_component
cannot have their safety expressed statically in Rust's type system. Instead, they are runtime borrow checked.However, this runtime borrow checking is only there as a diagnostic aid; it will provide more useful fail-fast behaviour and stack traces, rather than the state corruption that would otherwise have to be debugged.
The user must still carefully consider how these APIs are used and ensure that they are not breaking any borrowing rules. Therefore, these functions should still be marked as
unsafe
, to more clearly communicate this responsibility.The text was updated successfully, but these errors were encountered: