Skip to content

Commit

Permalink
Rollup merge of #118333 - eduardosm:print-missing-target-features, r=…
Browse files Browse the repository at this point in the history
…est31

Print list of missing target features when calling a function with target features outside an unsafe block

Fixes #108680

Supersedes #109710. I used the same wording for the messages, but the implementation is different.

r? ``@est31``
  • Loading branch information
matthiaskrgr authored Nov 29, 2023
2 parents 074e699 + 6b066a9 commit e3970cb
Show file tree
Hide file tree
Showing 10 changed files with 374 additions and 123 deletions.
13 changes: 10 additions & 3 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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 @@ -39,10 +39,17 @@ pub enum UnsafetyViolationDetails {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith,
CallToFunctionWith {
/// Target features enabled in callee's `#[target_feature]` but missing in
/// caller's `#[target_feature]`.
missing: Vec<Symbol>,
/// Target features in `missing` that are enabled at compile time
/// (e.g., with `-C target-feature`).
build_enabled: Vec<Symbol>,
},
}

#[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
36 changes: 33 additions & 3 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down
80 changes: 67 additions & 13 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -393,15 +396,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 },
);
}
}
}
Expand Down Expand Up @@ -527,7 +544,7 @@ struct UnusedUnsafeWarning {
enclosing_unsafe: Option<UnusedUnsafeEnclosing>,
}

#[derive(Clone, Copy, PartialEq)]
#[derive(Clone, PartialEq)]
enum UnsafeOpKind {
CallToUnsafeFunction(Option<DefId>),
UseOfInlineAssembly,
Expand All @@ -538,7 +555,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<Symbol>,
/// Target features in `missing` that are enabled at compile time
/// (e.g., with `-C target-feature`).
build_enabled: Vec<Symbol>,
},
}

use UnsafeOpKind::*;
Expand Down Expand Up @@ -659,13 +684,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,
},
),
Expand Down Expand Up @@ -822,18 +856,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),
});
}
}
Expand Down
25 changes: 22 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<UnsafeNotInheritedLintNote>,
}
Expand Down Expand Up @@ -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<UnsafeNotInheritedNote>,
}

#[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<UnsafeNotInheritedNote>,
}
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_mir_transform/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,19 @@ mir_transform_requires_unsafe = {$details} is unsafe and requires unsafe {$op_in
}
.not_inherited = items do not inherit unsafety from separate enclosing items
mir_transform_target_feature_call_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}
mir_transform_target_feature_call_label = call to function with `#[target_feature]`
mir_transform_target_feature_call_note = can only be called if the required target features are available
mir_transform_target_feature_call_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]`
mir_transform_unaligned_packed_ref = reference to packed field is unaligned
.note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
Expand Down
29 changes: 22 additions & 7 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,19 +287,20 @@ 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)
if !self.violations.contains(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)| {
Safety::FnUnsafe => violations.into_iter().for_each(|violation| {
let mut violation = violation.clone();
violation.kind = UnsafetyViolationKind::UnsafeFn;
if !self.violations.contains(&violation) {
self.violations.push(violation)
Expand Down Expand Up @@ -367,9 +368,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: Vec<_> = callee_features
.iter()
.copied()
.filter(|feature| !self_features.contains(feature))
.collect();
let build_enabled = self
.tcx
.sess
.target_features
.iter()
.copied()
.filter(|feature| missing.contains(feature))
.collect();
self.require_unsafe(
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToFunctionWith,
UnsafetyViolationDetails::CallToFunctionWith { missing, build_enabled },
)
}
}
Expand Down Expand Up @@ -528,8 +542,9 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
// Only suggest wrapping the entire function body in an unsafe block once
let mut suggest_unsafe_block = true;

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, ref details } in violations.iter() {
let details =
errors::RequiresUnsafeDetail { violation: details.clone(), span: source_info.span };

match kind {
UnsafetyViolationKind::General => {
Expand Down
Loading

0 comments on commit e3970cb

Please sign in to comment.