Rollup merge of rust-lang#126019 - tbu-:pr_unsafe_env_fixme, r=fee1-dead

Add TODO comment to unsafe env modification

Addresses rust-lang#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:
- rust-lang#124866

r? `@RalfJung`

Author: compiler-errors

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -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,
@@ -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(),
                         },

compiler/rustc_mir_build/src/errors.rs

@@ -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 = " }}")]

src/tools/tidy/src/style.rs

@@ -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",
                     )

tests/ui/rust-2024/unsafe-env-suggestion.fixed

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

tests/ui/rust-2024/unsafe-env-suggestion.stderr

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