From 6b066a9305f03425c6627c16419d0d7197a4c526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Tue, 28 Nov 2023 20:37:02 +0100 Subject: [PATCH] thir-unsafeck: print list of missing target features when calling a function with target features outside an unsafe block --- compiler/rustc_mir_build/messages.ftl | 36 ++++++++- .../rustc_mir_build/src/check_unsafety.rs | 80 ++++++++++++++++--- compiler/rustc_mir_build/src/errors.rs | 25 +++++- .../safe-calls.mir.stderr | 21 ++++- .../rfc-2396-target_feature-11/safe-calls.rs | 9 +++ .../safe-calls.thir.stderr | 48 ++++++++--- 6 files changed, 187 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 7dd0e7d4b92..c8d6c2114e9 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -30,12 +30,32 @@ mir_build_borrow_of_moved_value = borrow of moved value mir_build_call_to_fn_with_requires_unsafe = call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block - .note = can only be called if the required target features are available + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` .label = call to function with `#[target_feature]` mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe function or block - .note = can only be called if the required target features are available + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` .label = call to function with `#[target_feature]` mir_build_call_to_unsafe_fn_requires_unsafe = @@ -330,7 +350,17 @@ mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_uns mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe = call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block (error E0133) - .note = can only be called if the required target features are available + .help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count -> + [1] feature + *[count] features + }: {$missing_target_features} + .note = the {$build_target_features} target {$build_target_features_count -> + [1] feature + *[count] features + } being enabled in the build configuration does not remove the requirement to list {$build_target_features_count -> + [1] it + *[count] them + } in `#[target_feature]` .label = call to function with `#[target_feature]` mir_build_unsafe_op_in_unsafe_fn_call_to_unsafe_fn_requires_unsafe = diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index db0efd1d12e..2186fdf8dc2 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -1,7 +1,10 @@ +use std::borrow::Cow; + use crate::build::ExprCategory; use crate::errors::*; use rustc_middle::thir::visit::{self, Visitor}; +use rustc_errors::DiagnosticArgValue; use rustc_hir as hir; use rustc_middle::mir::BorrowKind; use rustc_middle::thir::*; @@ -392,15 +395,29 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { // the call requires `unsafe`. Don't check this on wasm // targets, though. For more information on wasm see the // is_like_wasm check in hir_analysis/src/collect.rs + let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features; if !self.tcx.sess.target.options.is_like_wasm - && !self - .tcx - .codegen_fn_attrs(func_did) - .target_features + && !callee_features .iter() .all(|feature| self.body_target_features.contains(feature)) { - self.requires_unsafe(expr.span, CallToFunctionWith(func_did)); + let missing: Vec<_> = callee_features + .iter() + .copied() + .filter(|feature| !self.body_target_features.contains(feature)) + .collect(); + let build_enabled = self + .tcx + .sess + .target_features + .iter() + .copied() + .filter(|feature| missing.contains(feature)) + .collect(); + self.requires_unsafe( + expr.span, + CallToFunctionWith { function: func_did, missing, build_enabled }, + ); } } } @@ -526,7 +543,7 @@ struct UnusedUnsafeWarning { enclosing_unsafe: Option, } -#[derive(Clone, Copy, PartialEq)] +#[derive(Clone, PartialEq)] enum UnsafeOpKind { CallToUnsafeFunction(Option), UseOfInlineAssembly, @@ -537,7 +554,15 @@ enum UnsafeOpKind { AccessToUnionField, MutationOfLayoutConstrainedField, BorrowOfLayoutConstrainedField, - CallToFunctionWith(DefId), + CallToFunctionWith { + function: DefId, + /// Target features enabled in callee's `#[target_feature]` but missing in + /// caller's `#[target_feature]`. + missing: Vec, + /// Target features in `missing` that are enabled at compile time + /// (e.g., with `-C target-feature`). + build_enabled: Vec, + }, } use UnsafeOpKind::*; @@ -658,13 +683,22 @@ impl UnsafeOpKind { unsafe_not_inherited_note, }, ), - CallToFunctionWith(did) => tcx.emit_spanned_lint( + CallToFunctionWith { function, missing, build_enabled } => tcx.emit_spanned_lint( UNSAFE_OP_IN_UNSAFE_FN, hir_id, span, UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe { span, - function: &with_no_trimmed_paths!(tcx.def_path_str(*did)), + function: &with_no_trimmed_paths!(tcx.def_path_str(*function)), + missing_target_features: DiagnosticArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + missing_target_features_count: missing.len(), + note: if build_enabled.is_empty() { None } else { Some(()) }, + build_target_features: DiagnosticArgValue::StrListSepByAnd( + build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + build_target_features_count: build_enabled.len(), unsafe_not_inherited_note, }, ), @@ -821,18 +855,38 @@ impl UnsafeOpKind { unsafe_not_inherited_note, }); } - CallToFunctionWith(did) if unsafe_op_in_unsafe_fn_allowed => { + CallToFunctionWith { function, missing, build_enabled } + if unsafe_op_in_unsafe_fn_allowed => + { tcx.sess.emit_err(CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed { span, + missing_target_features: DiagnosticArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + missing_target_features_count: missing.len(), + note: if build_enabled.is_empty() { None } else { Some(()) }, + build_target_features: DiagnosticArgValue::StrListSepByAnd( + build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + build_target_features_count: build_enabled.len(), unsafe_not_inherited_note, - function: &tcx.def_path_str(*did), + function: &tcx.def_path_str(*function), }); } - CallToFunctionWith(did) => { + CallToFunctionWith { function, missing, build_enabled } => { tcx.sess.emit_err(CallToFunctionWithRequiresUnsafe { span, + missing_target_features: DiagnosticArgValue::StrListSepByAnd( + missing.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + missing_target_features_count: missing.len(), + note: if build_enabled.is_empty() { None } else { Some(()) }, + build_target_features: DiagnosticArgValue::StrListSepByAnd( + build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(), + ), + build_target_features_count: build_enabled.len(), unsafe_not_inherited_note, - function: &tcx.def_path_str(*did), + function: &tcx.def_path_str(*function), }); } } diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 418f9bb9de9..8d2a559e73c 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -2,6 +2,7 @@ use crate::{ fluent_generated as fluent, thir::pattern::{deconstruct_pat::WitnessPat, MatchCheckCtxt}, }; +use rustc_errors::DiagnosticArgValue; use rustc_errors::{ error_code, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic, MultiSpan, SubdiagnosticMessage, @@ -124,11 +125,17 @@ pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe { #[derive(LintDiagnostic)] #[diag(mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe)] -#[note] +#[help] pub struct UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe<'a> { #[label] pub span: Span, pub function: &'a str, + pub missing_target_features: DiagnosticArgValue<'a>, + pub missing_target_features_count: usize, + #[note] + pub note: Option<()>, + pub build_target_features: DiagnosticArgValue<'a>, + pub build_target_features_count: usize, #[subdiagnostic] pub unsafe_not_inherited_note: Option, } @@ -369,24 +376,36 @@ pub struct BorrowOfLayoutConstrainedFieldRequiresUnsafeUnsafeOpInUnsafeFnAllowed #[derive(Diagnostic)] #[diag(mir_build_call_to_fn_with_requires_unsafe, code = "E0133")] -#[note] +#[help] pub struct CallToFunctionWithRequiresUnsafe<'a> { #[primary_span] #[label] pub span: Span, pub function: &'a str, + pub missing_target_features: DiagnosticArgValue<'a>, + pub missing_target_features_count: usize, + #[note] + pub note: Option<()>, + pub build_target_features: DiagnosticArgValue<'a>, + pub build_target_features_count: usize, #[subdiagnostic] pub unsafe_not_inherited_note: Option, } #[derive(Diagnostic)] #[diag(mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed, code = "E0133")] -#[note] +#[help] pub struct CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed<'a> { #[primary_span] #[label] pub span: Span, pub function: &'a str, + pub missing_target_features: DiagnosticArgValue<'a>, + pub missing_target_features_count: usize, + #[note] + pub note: Option<()>, + pub build_target_features: DiagnosticArgValue<'a>, + pub build_target_features_count: usize, #[subdiagnostic] pub unsafe_not_inherited_note: Option, } diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.mir.stderr b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.mir.stderr index c925034b253..cabc475fa61 100644 --- a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.mir.stderr +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.mir.stderr @@ -91,6 +91,25 @@ LL | const _: () = sse2_and_fxsr(); = help: in order for the call to be safe, the context requires the following additional target features: sse2 and fxsr = note: the fxsr and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]` -error: aborting due to 11 previous errors +error: call to function with `#[target_feature]` is unsafe and requires unsafe block (error E0133) + --> $DIR/safe-calls.rs:82:5 + | +LL | sse2(); + | ^^^^^^ call to function with `#[target_feature]` + | + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` +note: an unsafe function restricts its caller, but its body is safe by default + --> $DIR/safe-calls.rs:81:1 + | +LL | unsafe fn needs_unsafe_block() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: the lint level is defined here + --> $DIR/safe-calls.rs:78:8 + | +LL | #[deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.rs b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.rs index db9cf97282a..f17dab269bc 100644 --- a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.rs +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.rs @@ -75,4 +75,13 @@ const _: () = sse2_and_fxsr(); //[mir]~^ ERROR call to function with `#[target_feature]` is unsafe //[thir]~^^ ERROR call to function `sse2_and_fxsr` with `#[target_feature]` is unsafe +#[deny(unsafe_op_in_unsafe_fn)] +#[target_feature(enable = "avx")] +#[target_feature(enable = "bmi2")] +unsafe fn needs_unsafe_block() { + sse2(); + //[mir]~^ ERROR call to function with `#[target_feature]` is unsafe + //[thir]~^^ ERROR call to function `sse2` with `#[target_feature]` is unsafe +} + fn main() {} diff --git a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.thir.stderr b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.thir.stderr index 28a7adb012b..13b58fde862 100644 --- a/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.thir.stderr +++ b/tests/ui/rfcs/rfc-2396-target_feature-11/safe-calls.thir.stderr @@ -4,7 +4,8 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req LL | sse2(); | ^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:31:5 @@ -12,7 +13,7 @@ error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and LL | avx_bmi2(); | ^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2 error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:34:5 @@ -20,7 +21,7 @@ error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsa LL | Quux.avx_bmi2(); | ^^^^^^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2 error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:41:5 @@ -28,7 +29,7 @@ error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and LL | avx_bmi2(); | ^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2 error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:44:5 @@ -36,7 +37,7 @@ error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsa LL | Quux.avx_bmi2(); | ^^^^^^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target features: avx and bmi2 error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:51:5 @@ -44,7 +45,8 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req LL | sse2(); | ^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:54:5 @@ -52,7 +54,7 @@ error[E0133]: call to function `avx_bmi2` with `#[target_feature]` is unsafe and LL | avx_bmi2(); | ^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: bmi2 error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:57:5 @@ -60,7 +62,7 @@ error[E0133]: call to function `Quux::avx_bmi2` with `#[target_feature]` is unsa LL | Quux.avx_bmi2(); | ^^^^^^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: bmi2 error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:65:5 @@ -68,7 +70,8 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req LL | sse2(); | ^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:70:15 @@ -76,7 +79,8 @@ error[E0133]: call to function `sse2` with `#[target_feature]` is unsafe and req LL | const _: () = sse2(); | ^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` error[E0133]: call to function `sse2_and_fxsr` with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:74:15 @@ -84,8 +88,28 @@ error[E0133]: call to function `sse2_and_fxsr` with `#[target_feature]` is unsaf LL | const _: () = sse2_and_fxsr(); | ^^^^^^^^^^^^^^^ call to function with `#[target_feature]` | - = note: can only be called if the required target features are available + = help: in order for the call to be safe, the context requires the following additional target features: sse2 and fxsr + = note: the fxsr and sse2 target features being enabled in the build configuration does not remove the requirement to list them in `#[target_feature]` -error: aborting due to 11 previous errors +error: call to function `sse2` with `#[target_feature]` is unsafe and requires unsafe block (error E0133) + --> $DIR/safe-calls.rs:82:5 + | +LL | sse2(); + | ^^^^^^ call to function with `#[target_feature]` + | + = help: in order for the call to be safe, the context requires the following additional target feature: sse2 + = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]` +note: an unsafe function restricts its caller, but its body is safe by default + --> $DIR/safe-calls.rs:81:1 + | +LL | unsafe fn needs_unsafe_block() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: the lint level is defined here + --> $DIR/safe-calls.rs:78:8 + | +LL | #[deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0133`.