Add MVP suggestion for unsafe_op_in_unsafe_fn
#112017
Conversation
|
r? @eholk (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
| @@ -10,6 +10,14 @@ note: the lint level is defined here | |||
| | | |||
| LL | #![deny(unsafe_op_in_unsafe_fn)] | |||
| | ^^^^^^^^^^^^^^^^^^^^^^ | |||
| help: consider wrapping the function in an unsafe block | |||
There was a problem hiding this comment.
Actually, there was one outstanding point:
I'm not sure that this help text is useful -- I assume it's not necessary for the suggestion to be emitted for us to show it to humans too?
I feel like something more human-applicable here would be better, along the lines of "help: an unsafe function restricts callers, but it's body is safe by default". But I'm struggling with something succinct here.
It's probably a good idea to update the text in E0133 with some more expansive commentary on the effects of unsafe_op_in_unsafe_fn in any case, since it currently says nothing about that case.
Any suggestions on whether to remove the message completely, or a better message to replace it with?
There was a problem hiding this comment.
The existing suggestion I gave: not providing a in-code suggestion and instead provide just the help text, e.g., "help: an unsafe function restricts callers, but it's body is safe by default".
Nemo157
force-pushed
the
unsafe-block-rustfix
branch
from
1ba07cb to
ae5f4cd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Nemo157
force-pushed
the
unsafe-block-rustfix
branch
from
3016a8e to
51e330a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Nemo157
force-pushed
the
unsafe-block-rustfix
branch
from
51e330a to
d0298c1
Compare
| @@ -55,6 +55,8 @@ mir_transform_unaligned_packed_ref = reference to packed field is unaligned | |||
| mir_transform_union_access_label = access to union field | |||
| mir_transform_union_access_note = the field may not be properly initialized: using uninitialized data will cause undefined behavior | |||
| mir_transform_unsafe_op_in_unsafe_fn = {$details} is unsafe and requires unsafe block (error E0133) | |||
| .suggestion = consider wrapping the function in an unsafe block | |||
There was a problem hiding this comment.
| .suggestion = consider wrapping the function in an unsafe block | |
| .suggestion = consider wrapping the function body in an unsafe block |
To me wrapping the function in an unsafe block would mean something like this, even though it's nonsensical.
unsafe {
unsafe fn foo() { ... }
}| @@ -130,6 +131,7 @@ impl RequiresUnsafeDetail { | |||
|
|
|||
| pub(crate) struct UnsafeOpInUnsafeFn { | |||
| pub details: RequiresUnsafeDetail, | |||
| pub suggest_unsafe_block: Option<(Span, Span, Span)>, | |||
There was a problem hiding this comment.
Maybe add a doc comment describing what each of the three spans means?
Nemo157 rebase notes: Migrated the changes to the lint into fluent
Nemo157
force-pushed
the
unsafe-block-rustfix
branch
from
d0298c1 to
802c1d5
Compare
|
@bors r=eholk |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (5683791): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 647.795s -> 648.13s (0.05%) |
…safe_fn, r=RalfJung Change `unsafe_op_in_unsafe_fn` to be `warn`-by-default from edition 2024 This was previously FCPed: rust-lang#71668 (comment) There were two blocking requirements: * Fix the `unused_unsafe` lint, done in rust-lang#100081 * Have `cargo fix` able to fix the lint, done in rust-lang#112017
…r=RalfJung Change `unsafe_op_in_unsafe_fn` to be `warn`-by-default from edition 2024 This was previously FCPed: rust-lang/rust#71668 (comment) There were two blocking requirements: * Fix the `unused_unsafe` lint, done in rust-lang/rust#100081 * Have `cargo fix` able to fix the lint, done in rust-lang/rust#112017
…r=RalfJung Change `unsafe_op_in_unsafe_fn` to be `warn`-by-default from edition 2024 This was previously FCPed: rust-lang/rust#71668 (comment) There were two blocking requirements: * Fix the `unused_unsafe` lint, done in rust-lang/rust#100081 * Have `cargo fix` able to fix the lint, done in rust-lang/rust#112017
…r=RalfJung Change `unsafe_op_in_unsafe_fn` to be `warn`-by-default from edition 2024 This was previously FCPed: rust-lang/rust#71668 (comment) There were two blocking requirements: * Fix the `unused_unsafe` lint, done in rust-lang/rust#100081 * Have `cargo fix` able to fix the lint, done in rust-lang/rust#112017
Rebase of #99827
cc tracking issue #71668
No real changes since the original PR, just migrated the new suggestion to use fluent messages and added a couple more testcases, AFAICT from the discussion there were no outstanding changes requested.