Skip to content

Commit 911a5ee

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#118333 - eduardosm:print-missing-target-features, r=est31
Print list of missing target features when calling a function with target features outside an unsafe block Fixes rust-lang#108680 Supersedes rust-lang#109710. I used the same wording for the messages, but the implementation is different. r? `@est31`
2 parents afe2d73 + 6b066a9 commit 911a5ee

File tree

10 files changed

+374
-123
lines changed

10 files changed

+374
-123
lines changed

Diff for: ‎compiler/rustc_middle/src/mir/query.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub enum UnsafetyViolationKind {
2727
UnsafeFn,
2828
}
2929

30-
#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
30+
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
3131
pub enum UnsafetyViolationDetails {
3232
CallToUnsafeFunction,
3333
UseOfInlineAssembly,
@@ -39,10 +39,17 @@ pub enum UnsafetyViolationDetails {
3939
AccessToUnionField,
4040
MutationOfLayoutConstrainedField,
4141
BorrowOfLayoutConstrainedField,
42-
CallToFunctionWith,
42+
CallToFunctionWith {
43+
/// Target features enabled in callee's `#[target_feature]` but missing in
44+
/// caller's `#[target_feature]`.
45+
missing: Vec<Symbol>,
46+
/// Target features in `missing` that are enabled at compile time
47+
/// (e.g., with `-C target-feature`).
48+
build_enabled: Vec<Symbol>,
49+
},
4350
}
4451

45-
#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
52+
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
4653
pub struct UnsafetyViolation {
4754
pub source_info: SourceInfo,
4855
pub lint_root: hir::HirId,

Diff for: ‎compiler/rustc_mir_build/messages.ftl

+33-3
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,32 @@ mir_build_borrow_of_moved_value = borrow of moved value
3030
3131
mir_build_call_to_fn_with_requires_unsafe =
3232
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block
33-
.note = can only be called if the required target features are available
33+
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
34+
[1] feature
35+
*[count] features
36+
}: {$missing_target_features}
37+
.note = the {$build_target_features} target {$build_target_features_count ->
38+
[1] feature
39+
*[count] features
40+
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
41+
[1] it
42+
*[count] them
43+
} in `#[target_feature]`
3444
.label = call to function with `#[target_feature]`
3545
3646
mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
3747
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe function or block
38-
.note = can only be called if the required target features are available
48+
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
49+
[1] feature
50+
*[count] features
51+
}: {$missing_target_features}
52+
.note = the {$build_target_features} target {$build_target_features_count ->
53+
[1] feature
54+
*[count] features
55+
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
56+
[1] it
57+
*[count] them
58+
} in `#[target_feature]`
3959
.label = call to function with `#[target_feature]`
4060
4161
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
330350
331351
mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe =
332352
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block (error E0133)
333-
.note = can only be called if the required target features are available
353+
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
354+
[1] feature
355+
*[count] features
356+
}: {$missing_target_features}
357+
.note = the {$build_target_features} target {$build_target_features_count ->
358+
[1] feature
359+
*[count] features
360+
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
361+
[1] it
362+
*[count] them
363+
} in `#[target_feature]`
334364
.label = call to function with `#[target_feature]`
335365
336366
mir_build_unsafe_op_in_unsafe_fn_call_to_unsafe_fn_requires_unsafe =

Diff for: ‎compiler/rustc_mir_build/src/check_unsafety.rs

+67-13
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
use std::borrow::Cow;
2+
13
use crate::build::ExprCategory;
24
use crate::errors::*;
35
use rustc_middle::thir::visit::{self, Visitor};
46

7+
use rustc_errors::DiagnosticArgValue;
58
use rustc_hir as hir;
69
use rustc_middle::mir::BorrowKind;
710
use rustc_middle::thir::*;
@@ -393,15 +396,29 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
393396
// the call requires `unsafe`. Don't check this on wasm
394397
// targets, though. For more information on wasm see the
395398
// is_like_wasm check in hir_analysis/src/collect.rs
399+
let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
396400
if !self.tcx.sess.target.options.is_like_wasm
397-
&& !self
398-
.tcx
399-
.codegen_fn_attrs(func_did)
400-
.target_features
401+
&& !callee_features
401402
.iter()
402403
.all(|feature| self.body_target_features.contains(feature))
403404
{
404-
self.requires_unsafe(expr.span, CallToFunctionWith(func_did));
405+
let missing: Vec<_> = callee_features
406+
.iter()
407+
.copied()
408+
.filter(|feature| !self.body_target_features.contains(feature))
409+
.collect();
410+
let build_enabled = self
411+
.tcx
412+
.sess
413+
.target_features
414+
.iter()
415+
.copied()
416+
.filter(|feature| missing.contains(feature))
417+
.collect();
418+
self.requires_unsafe(
419+
expr.span,
420+
CallToFunctionWith { function: func_did, missing, build_enabled },
421+
);
405422
}
406423
}
407424
}
@@ -527,7 +544,7 @@ struct UnusedUnsafeWarning {
527544
enclosing_unsafe: Option<UnusedUnsafeEnclosing>,
528545
}
529546

530-
#[derive(Clone, Copy, PartialEq)]
547+
#[derive(Clone, PartialEq)]
531548
enum UnsafeOpKind {
532549
CallToUnsafeFunction(Option<DefId>),
533550
UseOfInlineAssembly,
@@ -538,7 +555,15 @@ enum UnsafeOpKind {
538555
AccessToUnionField,
539556
MutationOfLayoutConstrainedField,
540557
BorrowOfLayoutConstrainedField,
541-
CallToFunctionWith(DefId),
558+
CallToFunctionWith {
559+
function: DefId,
560+
/// Target features enabled in callee's `#[target_feature]` but missing in
561+
/// caller's `#[target_feature]`.
562+
missing: Vec<Symbol>,
563+
/// Target features in `missing` that are enabled at compile time
564+
/// (e.g., with `-C target-feature`).
565+
build_enabled: Vec<Symbol>,
566+
},
542567
}
543568

544569
use UnsafeOpKind::*;
@@ -659,13 +684,22 @@ impl UnsafeOpKind {
659684
unsafe_not_inherited_note,
660685
},
661686
),
662-
CallToFunctionWith(did) => tcx.emit_spanned_lint(
687+
CallToFunctionWith { function, missing, build_enabled } => tcx.emit_spanned_lint(
663688
UNSAFE_OP_IN_UNSAFE_FN,
664689
hir_id,
665690
span,
666691
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
667692
span,
668-
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
693+
function: &with_no_trimmed_paths!(tcx.def_path_str(*function)),
694+
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
695+
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
696+
),
697+
missing_target_features_count: missing.len(),
698+
note: if build_enabled.is_empty() { None } else { Some(()) },
699+
build_target_features: DiagnosticArgValue::StrListSepByAnd(
700+
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
701+
),
702+
build_target_features_count: build_enabled.len(),
669703
unsafe_not_inherited_note,
670704
},
671705
),
@@ -822,18 +856,38 @@ impl UnsafeOpKind {
822856
unsafe_not_inherited_note,
823857
});
824858
}
825-
CallToFunctionWith(did) if unsafe_op_in_unsafe_fn_allowed => {
859+
CallToFunctionWith { function, missing, build_enabled }
860+
if unsafe_op_in_unsafe_fn_allowed =>
861+
{
826862
tcx.sess.emit_err(CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed {
827863
span,
864+
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
865+
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
866+
),
867+
missing_target_features_count: missing.len(),
868+
note: if build_enabled.is_empty() { None } else { Some(()) },
869+
build_target_features: DiagnosticArgValue::StrListSepByAnd(
870+
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
871+
),
872+
build_target_features_count: build_enabled.len(),
828873
unsafe_not_inherited_note,
829-
function: &tcx.def_path_str(*did),
874+
function: &tcx.def_path_str(*function),
830875
});
831876
}
832-
CallToFunctionWith(did) => {
877+
CallToFunctionWith { function, missing, build_enabled } => {
833878
tcx.sess.emit_err(CallToFunctionWithRequiresUnsafe {
834879
span,
880+
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
881+
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
882+
),
883+
missing_target_features_count: missing.len(),
884+
note: if build_enabled.is_empty() { None } else { Some(()) },
885+
build_target_features: DiagnosticArgValue::StrListSepByAnd(
886+
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
887+
),
888+
build_target_features_count: build_enabled.len(),
835889
unsafe_not_inherited_note,
836-
function: &tcx.def_path_str(*did),
890+
function: &tcx.def_path_str(*function),
837891
});
838892
}
839893
}

Diff for: ‎compiler/rustc_mir_build/src/errors.rs

+22-3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::{
22
fluent_generated as fluent,
33
thir::pattern::{deconstruct_pat::WitnessPat, MatchCheckCtxt},
44
};
5+
use rustc_errors::DiagnosticArgValue;
56
use rustc_errors::{
67
error_code, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
78
Handler, IntoDiagnostic, MultiSpan, SubdiagnosticMessage,
@@ -124,11 +125,17 @@ pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
124125

125126
#[derive(LintDiagnostic)]
126127
#[diag(mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe)]
127-
#[note]
128+
#[help]
128129
pub struct UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe<'a> {
129130
#[label]
130131
pub span: Span,
131132
pub function: &'a str,
133+
pub missing_target_features: DiagnosticArgValue<'a>,
134+
pub missing_target_features_count: usize,
135+
#[note]
136+
pub note: Option<()>,
137+
pub build_target_features: DiagnosticArgValue<'a>,
138+
pub build_target_features_count: usize,
132139
#[subdiagnostic]
133140
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
134141
}
@@ -369,24 +376,36 @@ pub struct BorrowOfLayoutConstrainedFieldRequiresUnsafeUnsafeOpInUnsafeFnAllowed
369376

370377
#[derive(Diagnostic)]
371378
#[diag(mir_build_call_to_fn_with_requires_unsafe, code = "E0133")]
372-
#[note]
379+
#[help]
373380
pub struct CallToFunctionWithRequiresUnsafe<'a> {
374381
#[primary_span]
375382
#[label]
376383
pub span: Span,
377384
pub function: &'a str,
385+
pub missing_target_features: DiagnosticArgValue<'a>,
386+
pub missing_target_features_count: usize,
387+
#[note]
388+
pub note: Option<()>,
389+
pub build_target_features: DiagnosticArgValue<'a>,
390+
pub build_target_features_count: usize,
378391
#[subdiagnostic]
379392
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedNote>,
380393
}
381394

382395
#[derive(Diagnostic)]
383396
#[diag(mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed, code = "E0133")]
384-
#[note]
397+
#[help]
385398
pub struct CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed<'a> {
386399
#[primary_span]
387400
#[label]
388401
pub span: Span,
389402
pub function: &'a str,
403+
pub missing_target_features: DiagnosticArgValue<'a>,
404+
pub missing_target_features_count: usize,
405+
#[note]
406+
pub note: Option<()>,
407+
pub build_target_features: DiagnosticArgValue<'a>,
408+
pub build_target_features_count: usize,
390409
#[subdiagnostic]
391410
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedNote>,
392411
}

Diff for: ‎compiler/rustc_mir_transform/messages.ftl

+12-1
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,19 @@ mir_transform_requires_unsafe = {$details} is unsafe and requires unsafe {$op_in
4242
}
4343
.not_inherited = items do not inherit unsafety from separate enclosing items
4444
45+
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 ->
46+
[1] feature
47+
*[count] features
48+
}: {$missing_target_features}
49+
4550
mir_transform_target_feature_call_label = call to function with `#[target_feature]`
46-
mir_transform_target_feature_call_note = can only be called if the required target features are available
51+
mir_transform_target_feature_call_note = the {$build_target_features} target {$build_target_features_count ->
52+
[1] feature
53+
*[count] features
54+
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
55+
[1] it
56+
*[count] them
57+
} in `#[target_feature]`
4758
4859
mir_transform_unaligned_packed_ref = reference to packed field is unaligned
4960
.note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses

Diff for: ‎compiler/rustc_mir_transform/src/check_unsafety.rs

+22-7
Original file line numberDiff line numberDiff line change
@@ -287,19 +287,20 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> {
287287
.safety;
288288
match safety {
289289
// `unsafe` blocks are required in safe code
290-
Safety::Safe => violations.into_iter().for_each(|&violation| {
290+
Safety::Safe => violations.into_iter().for_each(|violation| {
291291
match violation.kind {
292292
UnsafetyViolationKind::General => {}
293293
UnsafetyViolationKind::UnsafeFn => {
294294
bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
295295
}
296296
}
297-
if !self.violations.contains(&violation) {
298-
self.violations.push(violation)
297+
if !self.violations.contains(violation) {
298+
self.violations.push(violation.clone())
299299
}
300300
}),
301301
// With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s
302-
Safety::FnUnsafe => violations.into_iter().for_each(|&(mut violation)| {
302+
Safety::FnUnsafe => violations.into_iter().for_each(|violation| {
303+
let mut violation = violation.clone();
303304
violation.kind = UnsafetyViolationKind::UnsafeFn;
304305
if !self.violations.contains(&violation) {
305306
self.violations.push(violation)
@@ -367,9 +368,22 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> {
367368

368369
// Is `callee_features` a subset of `calling_features`?
369370
if !callee_features.iter().all(|feature| self_features.contains(feature)) {
371+
let missing: Vec<_> = callee_features
372+
.iter()
373+
.copied()
374+
.filter(|feature| !self_features.contains(feature))
375+
.collect();
376+
let build_enabled = self
377+
.tcx
378+
.sess
379+
.target_features
380+
.iter()
381+
.copied()
382+
.filter(|feature| missing.contains(feature))
383+
.collect();
370384
self.require_unsafe(
371385
UnsafetyViolationKind::General,
372-
UnsafetyViolationDetails::CallToFunctionWith,
386+
UnsafetyViolationDetails::CallToFunctionWith { missing, build_enabled },
373387
)
374388
}
375389
}
@@ -528,8 +542,9 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
528542
// Only suggest wrapping the entire function body in an unsafe block once
529543
let mut suggest_unsafe_block = true;
530544

531-
for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() {
532-
let details = errors::RequiresUnsafeDetail { violation: details, span: source_info.span };
545+
for &UnsafetyViolation { source_info, lint_root, kind, ref details } in violations.iter() {
546+
let details =
547+
errors::RequiresUnsafeDetail { violation: details.clone(), span: source_info.span };
533548

534549
match kind {
535550
UnsafetyViolationKind::General => {

0 commit comments

Comments
 (0)