From 39c3b86eaa756cf3b8a8957abae7fd029196f90d Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Tue, 11 Jun 2024 19:34:53 +0200 Subject: [PATCH 1/2] Add a `ignore-tidy-todo` to ignore the tidy TODO comment check --- src/tools/tidy/src/style.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 64cc227762055..2d3888ec75f25 100644 --- a/src/tools/tidy/src/style.rs +++ b/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", ) From 4f5fb3126f24e2dbe401430c2b6961941be181f9 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Wed, 5 Jun 2024 11:06:41 +0200 Subject: [PATCH 2/2] Add TODO comment to unsafe env modification Addresses https://github.com/rust-lang/rust/pull/124636#issuecomment-2132119534. 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. --- compiler/rustc_mir_build/src/check_unsafety.rs | 3 +++ compiler/rustc_mir_build/src/errors.rs | 5 +++++ tests/ui/rust-2024/unsafe-env-suggestion.fixed | 2 ++ tests/ui/rust-2024/unsafe-env-suggestion.stderr | 10 ++++++---- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index d781fb1c297d9..a65586ccdb7e7 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/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(), }, diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index cf324c03dc9f2..3bd2e47976b57 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/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 = " }}")] diff --git a/tests/ui/rust-2024/unsafe-env-suggestion.fixed b/tests/ui/rust-2024/unsafe-env-suggestion.fixed index d9c738edfacbd..1f3d2dc0a31e8 100644 --- a/tests/ui/rust-2024/unsafe-env-suggestion.fixed +++ b/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 diff --git a/tests/ui/rust-2024/unsafe-env-suggestion.stderr b/tests/ui/rust-2024/unsafe-env-suggestion.stderr index 90c91c2a474a6..7c12f4aa5ede2 100644 --- a/tests/ui/rust-2024/unsafe-env-suggestion.stderr +++ b/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 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