Skip to content

Commit

Permalink
fixed target_feature 1.1 should print the list of missing target feat…
Browse files Browse the repository at this point in the history
…ures
  • Loading branch information
rohaquinlop committed Jun 7, 2023
1 parent adc719d commit 73fd02f
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 39 deletions.
83 changes: 77 additions & 6 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -38,10 +38,81 @@ pub enum UnsafetyViolationDetails {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith,
CallToFunctionWith { missing_features: Vec<Symbol>, target_features: Vec<Symbol> },
}

impl UnsafetyViolationDetails {
fn missing_features(&self) -> Vec<Symbol> {
match self {
UnsafetyViolationDetails::CallToFunctionWith {
missing_features,
target_features: _,
} => missing_features.to_vec(),
_ => Vec::<Symbol>::new(),
}
}

fn target_features(&self) -> Vec<Symbol> {
match self {
UnsafetyViolationDetails::CallToFunctionWith {
missing_features: _,
target_features,
} => target_features.to_vec(),
_ => Vec::<Symbol>::new(),
}
}

pub fn note_missing_features(&self) -> Option<String> {
let target_features_symbol = self.target_features();
let target_features =
target_features_symbol.iter().map(|f| f.as_str()).collect::<Vec<&str>>();

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<String> {
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::<Vec<&str>>()
.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 {
Expand Down Expand Up @@ -91,15 +162,15 @@ 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",
),
}
}
}

#[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,
Expand Down
50 changes: 36 additions & 14 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -287,22 +288,23 @@ 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 => {
bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
}
}
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 => {}
Expand Down Expand Up @@ -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::<Vec<Symbol>>();
let target_features = self
.tcx
.sess
.target_features
.iter()
.filter(|feature| missing_features.contains(feature))
.cloned()
.collect::<Vec<Symbol>>();
self.require_unsafe(
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToFunctionWith,
UnsafetyViolationDetails::CallToFunctionWith { missing_features, target_features },
)
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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(),
},
),
}
}
Expand Down
59 changes: 50 additions & 9 deletions compiler/rustc_mir_transform/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ pub(crate) struct RequiresUnsafe {
pub details: RequiresUnsafeDetail,
pub enclosing: Option<Span>,
pub op_in_unsafe_fn_allowed: bool,
pub help: Option<String>,
pub note_missing_features: Option<String>,
}

// The primary message for this diagnostic should be '{$label} is unsafe and...',
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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
}
}
}

Expand All @@ -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<String>,
pub note_missing_features: Option<String>,
}

impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn {
Expand All @@ -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
}

Expand Down
Loading

0 comments on commit 73fd02f

Please sign in to comment.