Conversation
error: expected identifier, found keyword `use` --> src/main.rs:2:20 | 2 | #![clippy::safety::use(invariant::ValidPtr)] // 💡 | ^^^ expected identifier, found keyword
Co-authored-by: kennytm <kennytm@gmail.com>
Also * distinguish safety::requires and safety::checked * replace safety::r#use with safety::import * only allow uninhabited enum as tag item
|
rust-lang/rust-clippy#11600 was a related effort on the clippy side that stalled on inactivity, though not specific to |
Thanks for the link. I’ve expanded the “Alternatives” section with a side-by-side comparison. |
|
I've left some comments in rust-for-linux channel on zulip:
src: #rust-for-linux > Provide the meaning of "Valid" to Understand. @ 💬 |
|
|
||
| #[safety::checked( | ||
| aligned = "alignment of ptr has be adjusted", | ||
| valid_ptr, initialized = "delegated to the caller" |
There was a problem hiding this comment.
(As an enhanced example from my comment in #3458 (comment) of RFC unsafe fields )
I'm wondering that there should be a specific #[safety::delegated] attribute the same as #[safety::checked], but more handy and concise to use:
#[safety::requires(
// bad comments by mentioning things in release build, but that's what the current implementation is in libstd
no_more_than_cap = "Caller must ensure the new_len <= cap (in release mode)",
valid_T_all_in_length_range = "Please also make sure all T in 0..=new_len are valid, since this call can't ensure this!"
)]
unsafe fn set_len(&mut self, new_len: usize) {
debug_assert!(new_len <= self.capacity());
#[safety::delegated(
no_more_than_cap, valid_T_all_in_length_range
)]
unsafe { self.len = new_len }
}Tags in delegated are merged into checked, so both can coexist and work well.
What I mean handy is that we can omit definition texts in such context, because safety requirements are first introduced on unsafe field declaration or unsafe function being called, so it's verbose to duplicate them when without extra context attached:
struct Vec<T> {
#[safety::requires(
no_more_than_cap = "0 <= len <= cap",
valid_T_all_in_length_range = "all elements in the Vec<T> between 0 and len are valid T"
)]
unsafe len: usize,
...
}
// The same safety requirements as on `unsafe len` field are rendered here
// because safety tags need support from rustdoc.
#[safety::requires(no_more_than_cap, valid_T_all_in_length_range)]
unsafe fn set_len(&mut self, new_len: usize) {
debug_assert!(new_len <= self.capacity());
#[safety::delegated(no_more_than_cap, valid_T_all_in_length_range)]
unsafe { self.len = new_len }
}There was a problem hiding this comment.
Another reason to have #[safety::delegated] is that #[safety::requires] must incorporate all tags delegated from the fn body. Must means that a diagnostic happens if such one tag is missing.
checked won't have such delegation checks when listing delegated tags in requires.
There was a problem hiding this comment.
Paste a totally semantic-unsafety-oriented example given in my comment on RFC unsafe fields:
struct Vec<T> {
#[safety::requires(any {
good_on_read,
hazard_on_mut(valid_T_all_in_length_range, no_more_than_cap)
})]
unsafe len: usize,
}
pub fn len(&self) -> usize {
#[safety::checked(good_on_read)]
unsafe { self.len }
}
#[safety::requires(hazard_on_mut)]
unsafe fn set_len(&mut self, new_len: usize) {
#[safety::delegated(hazard_on_mut)]
unsafe { self.len = new_len }
}This is an improvement to #3842 (comment) as well.
(I don't really suggest supporting safety tags beyond visual unsafe operations too far though.)
There was a problem hiding this comment.
you should at least add the unsafe back to the len field, I don't think it is supposed that #[safety::requires] can be applicable on things declared safe.
There was a problem hiding this comment.
The snippet is more to clippy PR danger_not_accepted.
Safety tags for unsafe fields are already stated to be supported.
There was a problem hiding this comment.
i mean your declaration of len is not an unsafe field at all, not whether about the attribute supports unsafe field or not.
struct Vec<T> {
#[safety::requires(any {
good_on_read,
hazard_on_mut(valid_T_all_in_length_range, no_more_than_cap)
})]
len: usize,
//^ NO UNSAFE IN SIGHT
}There was a problem hiding this comment.
danger_not_accepted doesn't require unsafe.
But to avoid further unnecessary misinterpretation, I've just modified it the way you asked.
|
Allowing an unstructured safety comment like |
|
We presented a talk on safety tags last Sunday. The talk is in Chinese, but the slides are in English. The presentation introduced safety-tool, which standardizes safety requirements and annotations into a machine-readable attribute syntax. It checks unsafe function declarations and calls to ensure that safety properties are without any omissions. |
|
We have developed a Web-based interface for auditing safety properties, featuring
Current support extends to the Standard Library (core, alloc, and std) and Asterinas' ostd crate. Although the tag set is still preliminary, it effectively demonstrates the framework's visualization capabilities. The application is hosted as a static webpage, and the repo is here. |
|
Heads up that the unsafe fields RFC (which is referenced in this RFC) has now been merged. cc @jswrenn |
|
I just noticed a case where safety invariants in libcore changed in a semver-breaking manner as introduced in rust-lang/rust#141260 and stablized since 1.90: for read_volatile/write_volatile on latest stable vs an old version such as 1.88, the safety doc section differs -src must be valid for reads.
+src must be either valid for reads, or it must point to memory outside of all Rust allocations
+and reading from that memory must:
+* not trap, and
+* not cause any memory inside a Rust allocation to be modified.Since maintaining invariant semver is a core motivation for safety tags, relying solely on release notes is insufficient; automated tool checks are essential to track such changes effectively. -#[safety::requires(valid_ptr(src))]
+#[safety::requires(any(valid_ptr(src), not_rust_allocation(src)))] |
To be clear, the std change was not semver-breaking, it relaxed the necessary preconditions. Users that did: #[safety::checked(valid_ptr(ptr))]
unsafe { ptr.read_volatile() };Would get no warning both before and after Rust 1.88. |
Thanks for the clarification! There's a contradiction in semver for this case:
|
Summary
This RFC introduces a concise safety-comment convention for unsafe code in standard libraries:
tag every public unsafe function with
#[safety::requires]and call with#[safety::checked].Safety tags refine today’s safety-comment habits: a featherweight syntax that condenses every
requirement into a single, check-off reminder.
The following snippet compiles today if we enable enough nightly features, but we expect Clippy
and Rust-Analyzer to enforce tag checks and provide first-class IDE support.
Rendered