From 73fd02f30f897bfa3b1e23c9edffd462aeeabe38 Mon Sep 17 00:00:00 2001 From: Robin Hafid Date: Tue, 18 Apr 2023 10:56:11 -0500 Subject: [PATCH] fixed target_feature 1.1 should print the list of missing target features --- compiler/rustc_middle/src/mir/query.rs | 83 +++++++++++++++++-- .../rustc_mir_transform/src/check_unsafety.rs | 50 +++++++---- compiler/rustc_mir_transform/src/errors.rs | 59 +++++++++++-- .../safe-calls.mir.stderr | 24 +++--- 4 files changed, 177 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index a15c419da7aca..2e3013dbbdf4a 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -4,12 +4,12 @@ use crate::mir::ConstantKind; use crate::ty::{self, OpaqueHiddenType, Ty, TyCtxt}; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::UnordSet; -use rustc_errors::ErrorGuaranteed; +use rustc_errors::{pluralize, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; use rustc_index::bit_set::BitMatrix; use rustc_index::{Idx, IndexVec}; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; use rustc_target::abi::{FieldIdx, VariantIdx}; use smallvec::SmallVec; use std::cell::Cell; @@ -26,7 +26,7 @@ pub enum UnsafetyViolationKind { UnsafeFn, } -#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)] +#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)] pub enum UnsafetyViolationDetails { CallToUnsafeFunction, UseOfInlineAssembly, @@ -38,10 +38,81 @@ pub enum UnsafetyViolationDetails { AccessToUnionField, MutationOfLayoutConstrainedField, BorrowOfLayoutConstrainedField, - CallToFunctionWith, + CallToFunctionWith { missing_features: Vec, target_features: Vec }, } impl UnsafetyViolationDetails { + fn missing_features(&self) -> Vec { + match self { + UnsafetyViolationDetails::CallToFunctionWith { + missing_features, + target_features: _, + } => missing_features.to_vec(), + _ => Vec::::new(), + } + } + + fn target_features(&self) -> Vec { + match self { + UnsafetyViolationDetails::CallToFunctionWith { + missing_features: _, + target_features, + } => target_features.to_vec(), + _ => Vec::::new(), + } + } + + pub fn note_missing_features(&self) -> Option { + let target_features_symbol = self.target_features(); + let target_features = + target_features_symbol.iter().map(|f| f.as_str()).collect::>(); + + if !target_features.is_empty() { + let target_features_str = if target_features.len() > 1 { + let l_target_features = target_features[..target_features.len() - 1].join(", "); + let last_target_feature = target_features.last().unwrap(); + format!("{} and {}", l_target_features, last_target_feature) + } else { + target_features.last().unwrap().to_string() + }; + + Some(format!( + "the {} target feature{} being enabled in the build configuration does not remove the requirement to list {} in `#[target_feature]`", + target_features_str, + pluralize!(target_features.len()), + if target_features.len() == 1 { "it" } else { "them" }, + )) + } else { + None + } + } + + pub fn help_missing_features(&self) -> Option { + let missing_features = self.missing_features(); + + if !missing_features.is_empty() { + let missing_features_str = if missing_features.len() > 1 { + let l_missing_features = missing_features[..missing_features.len() - 1] + .iter() + .map(|feature| feature.as_str()) + .collect::>() + .join(", "); + let last_missing_feature = missing_features.last().unwrap().as_str(); + format!("{} and {}", l_missing_features, last_missing_feature) + } else { + missing_features.last().unwrap().to_string() + }; + + Some(format!( + "in order for the call to be safe, the context requires the following additional target feature{}: {}.", + pluralize!(missing_features.len()), + missing_features_str, + )) + } else { + None + } + } + pub fn description_and_note(&self) -> (&'static str, &'static str) { use UnsafetyViolationDetails::*; match self { @@ -91,7 +162,7 @@ impl UnsafetyViolationDetails { "references to fields of layout constrained fields lose the constraints. Coupled \ with interior mutability, the field can be changed to invalid values", ), - CallToFunctionWith => ( + CallToFunctionWith { missing_features: _, target_features: _ } => ( "call to function with `#[target_feature]`", "can only be called if the required target features are available", ), @@ -99,7 +170,7 @@ impl UnsafetyViolationDetails { } } -#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)] +#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)] pub struct UnsafetyViolation { pub source_info: SourceInfo, pub lint_root: hir::HirId, diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index 2f851cd1eb5f7..b14f4438a63dc 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -11,6 +11,7 @@ use rustc_middle::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; use rustc_session::lint::Level; +use rustc_span::Symbol; use std::ops::Bound; @@ -287,7 +288,7 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> { .safety; match safety { // `unsafe` blocks are required in safe code - Safety::Safe => violations.into_iter().for_each(|&violation| { + Safety::Safe => violations.into_iter().for_each(|violation| { match violation.kind { UnsafetyViolationKind::General => {} UnsafetyViolationKind::UnsafeFn => { @@ -295,14 +296,15 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> { } } if !self.violations.contains(&violation) { - self.violations.push(violation) + self.violations.push(violation.clone()) } }), // With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s - Safety::FnUnsafe => violations.into_iter().for_each(|&(mut violation)| { - violation.kind = UnsafetyViolationKind::UnsafeFn; - if !self.violations.contains(&violation) { - self.violations.push(violation) + Safety::FnUnsafe => violations.into_iter().for_each(|violation| { + let mut violation_copy = violation.clone(); + violation_copy.kind = UnsafetyViolationKind::UnsafeFn; + if !self.violations.contains(&violation_copy) { + self.violations.push(violation_copy) } }), Safety::BuiltinUnsafe => {} @@ -367,9 +369,22 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> { // Is `callee_features` a subset of `calling_features`? if !callee_features.iter().all(|feature| self_features.contains(feature)) { + let missing_features = callee_features + .iter() + .filter(|feature| !self_features.contains(feature)) + .cloned() + .collect::>(); + let target_features = self + .tcx + .sess + .target_features + .iter() + .filter(|feature| missing_features.contains(feature)) + .cloned() + .collect::>(); self.require_unsafe( UnsafetyViolationKind::General, - UnsafetyViolationDetails::CallToFunctionWith, + UnsafetyViolationDetails::CallToFunctionWith { missing_features, target_features }, ) } } @@ -526,13 +541,14 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { let UnsafetyCheckResult { violations, unused_unsafes, .. } = tcx.unsafety_check_result(def_id); - for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() { - let details = errors::RequiresUnsafeDetail { violation: details, span: source_info.span }; + for UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() { + let unsafe_details = + errors::RequiresUnsafeDetail { violation: details.clone(), span: source_info.span }; match kind { UnsafetyViolationKind::General => { - let op_in_unsafe_fn_allowed = unsafe_op_in_unsafe_fn_allowed(tcx, lint_root); - let note_non_inherited = tcx.hir().parent_iter(lint_root).find(|(id, node)| { + let op_in_unsafe_fn_allowed = unsafe_op_in_unsafe_fn_allowed(tcx, *lint_root); + let note_non_inherited = tcx.hir().parent_iter(*lint_root).find(|(id, node)| { if let Node::Expr(block) = node && let ExprKind::Block(block, _) = block.kind && let BlockCheckMode::UnsafeBlock(_) = block.rules @@ -555,15 +571,21 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { tcx.sess.emit_err(errors::RequiresUnsafe { span: source_info.span, enclosing, - details, + details: unsafe_details, op_in_unsafe_fn_allowed, + help: details.help_missing_features(), + note_missing_features: details.note_missing_features(), }); } UnsafetyViolationKind::UnsafeFn => tcx.emit_spanned_lint( UNSAFE_OP_IN_UNSAFE_FN, - lint_root, + *lint_root, source_info.span, - errors::UnsafeOpInUnsafeFn { details }, + errors::UnsafeOpInUnsafeFn { + details: unsafe_details, + help: details.help_missing_features(), + note_missing_features: details.note_missing_features(), + }, ), } } diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 22f71bb08516c..8332841891985 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/compiler/rustc_mir_transform/src/errors.rs @@ -49,6 +49,8 @@ pub(crate) struct RequiresUnsafe { pub details: RequiresUnsafeDetail, pub enclosing: Option, pub op_in_unsafe_fn_allowed: bool, + pub help: Option, + pub note_missing_features: Option, } // The primary message for this diagnostic should be '{$label} is unsafe and...', @@ -62,9 +64,27 @@ impl<'sess, G: EmissionGuarantee> IntoDiagnostic<'sess, G> for RequiresUnsafe { handler.struct_diagnostic(crate::fluent_generated::mir_transform_requires_unsafe); diag.code(rustc_errors::DiagnosticId::Error("E0133".to_string())); diag.set_span(self.span); - diag.span_label(self.span, self.details.label()); - diag.note(self.details.note()); - let desc = handler.eagerly_translate_to_string(self.details.label(), [].into_iter()); + diag.span_label(self.span, self.details.clone().label()); + if let Some(help) = self.help { + diag.help(help); + } + + match self.details.violation { + UnsafetyViolationDetails::CallToFunctionWith { + missing_features: _, + target_features: _, + } => { + if let Some(note_missing_features) = self.note_missing_features { + diag.note(note_missing_features); + } + } + _ => { + diag.note(self.details.clone().note()); + } + } + + let desc = + handler.eagerly_translate_to_string(self.details.clone().label(), [].into_iter()); diag.set_arg("details", desc); diag.set_arg("op_in_unsafe_fn_allowed", self.op_in_unsafe_fn_allowed); if let Some(sp) = self.enclosing { @@ -74,7 +94,7 @@ impl<'sess, G: EmissionGuarantee> IntoDiagnostic<'sess, G> for RequiresUnsafe { } } -#[derive(Copy, Clone)] +#[derive(Clone)] pub(crate) struct RequiresUnsafeDetail { pub span: Span, pub violation: UnsafetyViolationDetails, @@ -100,7 +120,9 @@ impl RequiresUnsafeDetail { BorrowOfLayoutConstrainedField => { crate::fluent_generated::mir_transform_mutation_layout_constrained_borrow_note } - CallToFunctionWith => crate::fluent_generated::mir_transform_target_feature_call_note, + CallToFunctionWith { missing_features: _, target_features: _ } => { + crate::fluent_generated::mir_transform_target_feature_call_note + } } } @@ -123,13 +145,17 @@ impl RequiresUnsafeDetail { BorrowOfLayoutConstrainedField => { crate::fluent_generated::mir_transform_mutation_layout_constrained_borrow_label } - CallToFunctionWith => crate::fluent_generated::mir_transform_target_feature_call_label, + CallToFunctionWith { missing_features: _, target_features: _ } => { + crate::fluent_generated::mir_transform_target_feature_call_label + } } } } pub(crate) struct UnsafeOpInUnsafeFn { pub details: RequiresUnsafeDetail, + pub help: Option, + pub note_missing_features: Option, } impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn { @@ -141,10 +167,25 @@ impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn { let desc = diag .handler() .expect("lint should not yet be emitted") - .eagerly_translate_to_string(self.details.label(), [].into_iter()); + .eagerly_translate_to_string(self.details.clone().label(), [].into_iter()); diag.set_arg("details", desc); - diag.span_label(self.details.span, self.details.label()); - diag.note(self.details.note()); + diag.span_label(self.details.clone().span, self.details.clone().label()); + if let Some(help) = self.help { + diag.help(help); + } + match self.details.violation { + UnsafetyViolationDetails::CallToFunctionWith { + missing_features: _, + target_features: _, + } => { + if let Some(note_missing_features) = self.note_missing_features { + diag.note(note_missing_features); + } + } + _ => { + diag.note(self.details.note()); + } + } diag } 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 0ef7b8b09f11f..3a8e990751223 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 @@ -4,7 +4,8 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:26:5 @@ -12,7 +13,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:29:5 @@ -20,7 +21,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:36:5 @@ -28,7 +29,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:39:5 @@ -36,7 +37,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:46:5 @@ -44,7 +45,8 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:49:5 @@ -52,7 +54,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:52:5 @@ -60,7 +62,7 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:60:5 @@ -68,7 +70,8 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u 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 with `#[target_feature]` is unsafe and requires unsafe function or block --> $DIR/safe-calls.rs:65:18 @@ -76,7 +79,8 @@ error[E0133]: call to function with `#[target_feature]` is unsafe and requires u LL | const name: () = 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: aborting due to 10 previous errors