target_feature 1.1 error should print the list of missing target features

[est31]

Given the following code:

#![feature(target_feature_11)]

#[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
pub fn foo() {}

#[target_feature(enable = "sse")]
fn bar() {
    foo();
}

The current output is:

error[E0133]: call to function with `#[target_feature]` is unsafe and requires unsafe function or block
 --> src/main.rs:8:5
  |
8 |     foo();
  |     ^^^^^ call to function with `#[target_feature]`
  |
  = note: can only be called if the required target features are available

The note is easy to be misinterpreted: the function can also be called if the target features are not available, but it requires an unsafe annotation.

I think it would be nice for the note to list the missing target features. In this case, this would be sse2 and avx.
If one or more of the missing target features are enabled in the current build config, like via -C target-feature or -C target-cpu, or because the default is just that, then it should acknowledge this situation and explain that a #[target_feature] is still required.

So I'd imagine something like:

error[E0133]: call to function with `#[target_feature]` is unsafe and requires unsafe function or block
 --> src/main.rs:8:5
  |
8 |     foo();
  |     ^^^^^ call to function with `#[target_feature]`
  |
  = help: in order for the call to be safe, the context requires the following additional target features: sse2, avx
  = note: the sse2 target feature being enabled in the build configuration does not remove the requirement to list it in `#[target_feature]`

---

TLDR: I ask for two things:

1. removing the note in the general case (I think what it contains is already said in the main error msg), but if there are missing target features that are enabled in the build configuration, emit a note to point out that even though they are enabled, this has no influence on #[target_feature] (it's two different systems; see my example for the proposed wording)
2. adding a help that lists the missing target features (ideally if too many like more than 5 are missing, then cut the list to keep the error message short ("avx, sse2, sse, aes, foobar, and 3 more"))

cc #69098

@rustbot label A-diagnostics F-target_feature_11

[LeSeulArtichaut]

This seems like a nice first issue to me, I can mentor someone who would want to pick this up.

Mentoring instructions

The error originates from

rust/compiler/rustc_mir_transform/src/check_unsafety.rs

Lines 363 to 373 in 18caf88

	let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
	// The body might be a constant, so it doesn't have codegen attributes.
	let self_features = &self.tcx.body_codegen_attrs(self.body_did.to_def_id()).target_features;
	// Is `callee_features` a subset of `calling_features`?
	if !callee_features.iter().all(|feature| self_features.contains(feature)) {
	self.require_unsafe(
	UnsafetyViolationKind::General,
	UnsafetyViolationDetails::CallToFunctionWith,
	)
	}

The missing features should probably be added as additional context to UnsafetyViolationDetails::CallToFunctionWith.

The error is then emitted in

rust/compiler/rustc_mir_transform/src/check_unsafety.rs

Lines 557 to 611 in 18caf88

	for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() {
	let (description, note) = details.description_and_note();
	match kind {
	UnsafetyViolationKind::General => {
	// once
	let unsafe_fn_msg = if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) {
	" function or"
	} else {
	""
	};
	let mut err = struct_span_err!(
	tcx.sess,
	source_info.span,
	E0133,
	"{} is unsafe and requires unsafe{} block",
	description,
	unsafe_fn_msg,
	);
	err.span_label(source_info.span, description).note(note);
	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
	{
	true
	}
	else if let Some(sig) = tcx.hir().fn_sig_by_hir_id(*id)
	&& sig.header.is_unsafe()
	{
	true
	} else {
	false
	}
	});
	if let Some((id, _)) = note_non_inherited {
	let span = tcx.hir().span(id);
	err.span_label(
	tcx.sess.source_map().guess_head_span(span),
	"items do not inherit unsafety from separate enclosing items",
	);
	}
	err.emit();
	}
	UnsafetyViolationKind::UnsafeFn => tcx.struct_span_lint_hir(
	UNSAFE_OP_IN_UNSAFE_FN,
	lint_root,
	source_info.span,
	format!("{} is unsafe and requires unsafe block (error E0133)", description,),
	|lint| lint.span_label(source_info.span, description).note(note),
	),
	}
	}

where it can be tweaked as required, through the usual DiagnosticBuilder methods, e.g. note.

@rustbot label: +E-easy +E-mentor

[KittyBorgX]

@rustbot claim

[KittyBorgX]

@LeSeulArtichaut Thank you for the opportunity!
If I am to understand it correctly, I just add a note in the 2nd file ( compiler/rustc_mir_transform/src/check_unsafety.rs) right?

[LeSeulArtichaut]

Basically, yes. Make sure to add it to both the hard error (in UnsafetyViolationKind::General => { /* ... */ }) and the unsafe-op-in-unsafe-fn lint (in UnsafetyViolationKind::UnsafeFn => { /* ... */ }).

[rohaquinlop]

@rustbot claim

[est31]

For greater clarity, I have amended the issue description with a TLDR consisting of a list of the changes I'd like there to be.