Skip to content

Commit

Permalink
Rollup merge of #126019 - tbu-:pr_unsafe_env_fixme, r=fee1-dead
Browse files Browse the repository at this point in the history
Add TODO comment to unsafe env modification

Addresses #124636 (comment).

I think that the diff display regresses a little, because it's no longer showing the `+` to show where the `unsafe {}` is added. I think it's still fine.

Tracking:
- #124866

r? `@RalfJung`
  • Loading branch information
compiler-errors committed Jun 12, 2024
2 parents 7133257 + 4f5fb31 commit 88984fe
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 5 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
if !span.at_least_rust_2024()
&& self.tcx.has_attr(id, sym::rustc_deprecated_safe_2024) =>
{
let sm = self.tcx.sess.source_map();
self.tcx.emit_node_span_lint(
DEPRECATED_SAFE,
self.hir_context,
Expand All @@ -105,6 +106,8 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
span,
function: with_no_trimmed_paths!(self.tcx.def_path_str(id)),
sub: CallToDeprecatedSafeFnRequiresUnsafeSub {
indent: sm.indentation_before(span).unwrap_or_default(),
start_of_line: sm.span_extend_to_line(span).shrink_to_lo(),
left: span.shrink_to_lo(),
right: span.shrink_to_hi(),
},
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ pub(crate) struct CallToDeprecatedSafeFnRequiresUnsafe {
#[derive(Subdiagnostic)]
#[multipart_suggestion(mir_build_suggestion, applicability = "machine-applicable")]
pub(crate) struct CallToDeprecatedSafeFnRequiresUnsafeSub {
pub(crate) indent: String,
#[suggestion_part(
code = "{indent}// TODO: Audit that the environment access only happens in single-threaded code.\n" // ignore-tidy-todo
)]
pub(crate) start_of_line: Span,
#[suggestion_part(code = "unsafe {{ ")]
pub(crate) left: Span,
#[suggestion_part(code = " }}")]
Expand Down
4 changes: 3 additions & 1 deletion src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,9 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(err, skip_cr, "CR character");
}
if filename != "style.rs" {
if trimmed.contains("TODO") {
// Allow using TODO in diagnostic suggestions by marking the
// relevant line with `// ignore-tidy-todo`.
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {
err(
"TODO is used for tasks that should be done before merging a PR; If you want to leave a message in the codebase use FIXME",
)
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/rust-2024/unsafe-env-suggestion.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ use std::env;

#[deny(unused_unsafe)]
fn main() {
// TODO: Audit that the environment access only happens in single-threaded code.
unsafe { env::set_var("FOO", "BAR") };
//~^ ERROR call to deprecated safe function
//~| WARN this is accepted in the current edition
// TODO: Audit that the environment access only happens in single-threaded code.
unsafe { env::remove_var("FOO") };
//~^ ERROR call to deprecated safe function
//~| WARN this is accepted in the current edition
Expand Down
10 changes: 6 additions & 4 deletions tests/ui/rust-2024/unsafe-env-suggestion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ LL | #![deny(deprecated_safe)]
| ^^^^^^^^^^^^^^^
help: you can wrap the call in an `unsafe` block if you can guarantee the code is only ever called from single-threaded code
|
LL | unsafe { env::set_var("FOO", "BAR") };
| ++++++++ +
LL + // TODO: Audit that the environment access only happens in single-threaded code.
LL ~ unsafe { env::set_var("FOO", "BAR") };
|

error: call to deprecated safe function `std::env::remove_var` is unsafe and requires unsafe block
--> $DIR/unsafe-env-suggestion.rs:12:5
Expand All @@ -26,8 +27,9 @@ LL | env::remove_var("FOO");
= note: for more information, see issue #27970 <https://github.com/rust-lang/rust/issues/27970>
help: you can wrap the call in an `unsafe` block if you can guarantee the code is only ever called from single-threaded code
|
LL | unsafe { env::remove_var("FOO") };
| ++++++++ +
LL + // TODO: Audit that the environment access only happens in single-threaded code.
LL ~ unsafe { env::remove_var("FOO") };
|

error: aborting due to 2 previous errors

0 comments on commit 88984fe

Please sign in to comment.