Skip to content

Commit

Permalink
Auto merge of #69274 - LeSeulArtichaut:target-feature-11, r=hanna-kruppe
Browse files Browse the repository at this point in the history
Implement RFC 2396: `#[target_feature]` 1.1

Tracking issue: #69098

r? @nikomatsakis
cc @gnzlbg @joshtriplett
  • Loading branch information
bors committed May 2, 2020
2 parents d20113d + 8d9f73a commit f05a524
Show file tree
Hide file tree
Showing 19 changed files with 392 additions and 23 deletions.
3 changes: 3 additions & 0 deletions src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,9 @@ declare_features! (
/// Allow negative trait implementations.
(active, negative_impls, "1.44.0", Some(68318), None),

/// Allows the use of `#[target_feature]` on safe functions.
(active, target_feature_11, "1.45.0", Some(69098), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand Down
23 changes: 22 additions & 1 deletion src/librustc_middle/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ use rustc_ast::ast;
use rustc_errors::{pluralize, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_target::spec::abi;

use std::borrow::Cow;
use std::fmt;
use std::ops::Deref;

#[derive(Clone, Copy, Debug, PartialEq, Eq, TypeFoldable)]
pub struct ExpectedFound<T> {
Expand Down Expand Up @@ -58,6 +60,8 @@ pub enum TypeError<'tcx> {
ConstMismatch(ExpectedFound<&'tcx ty::Const<'tcx>>),

IntrinsicCast,
/// Safe `#[target_feature]` functions are not assignable to safe function pointers.
TargetFeatureCast(DefId),
}

pub enum UnconstrainedNumeric {
Expand Down Expand Up @@ -183,6 +187,10 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
write!(f, "expected `{}`, found `{}`", values.expected, values.found)
}
IntrinsicCast => write!(f, "cannot coerce intrinsics to function pointers"),
TargetFeatureCast(_) => write!(
f,
"cannot coerce functions with `#[target_feature]` to safe function pointers"
),
ObjectUnsafeCoercion(_) => write!(f, "coercion to object-unsafe trait object"),
}
}
Expand All @@ -193,7 +201,8 @@ impl<'tcx> TypeError<'tcx> {
use self::TypeError::*;
match self {
CyclicTy(_) | UnsafetyMismatch(_) | Mismatch | AbiMismatch(_) | FixedArraySize(_)
| Sorts(_) | IntMismatch(_) | FloatMismatch(_) | VariadicMismatch(_) => false,
| Sorts(_) | IntMismatch(_) | FloatMismatch(_) | VariadicMismatch(_)
| TargetFeatureCast(_) => false,

Mutability
| TupleSize(_)
Expand Down Expand Up @@ -489,6 +498,18 @@ impl Trait for X {
);
}
}
TargetFeatureCast(def_id) => {
let attrs = self.get_attrs(*def_id);
let target_spans = attrs
.deref()
.iter()
.filter(|attr| attr.has_name(sym::target_feature))
.map(|attr| attr.span);
db.note(
"functions with `#[target_feature]` can only be coerced to `unsafe` function pointers"
);
db.span_labels(target_spans, "`#[target_feature]` added here");
}
_ => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_middle/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch),
ConstMismatch(ref x) => return tcx.lift(x).map(ConstMismatch),
IntrinsicCast => IntrinsicCast,
TargetFeatureCast(ref x) => TargetFeatureCast(*x),
ObjectUnsafeCoercion(ref x) => return tcx.lift(x).map(ObjectUnsafeCoercion),
})
}
Expand Down
26 changes: 25 additions & 1 deletion src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::util;

pub struct UnsafetyChecker<'a, 'tcx> {
body: &'a Body<'tcx>,
body_did: LocalDefId,
const_context: bool,
min_const_fn: bool,
violations: Vec<UnsafetyViolation>,
Expand All @@ -35,6 +36,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
const_context: bool,
min_const_fn: bool,
body: &'a Body<'tcx>,
body_did: LocalDefId,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
Expand All @@ -44,6 +46,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
}
Self {
body,
body_did,
const_context,
min_const_fn,
violations: vec![],
Expand Down Expand Up @@ -87,6 +90,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
UnsafetyViolationKind::GeneralAndConstFn,
)
}

if let ty::FnDef(func_id, _) = func_ty.kind {
self.check_target_features(func_id);
}
}
}
self.super_terminator(terminator, location);
Expand Down Expand Up @@ -436,6 +443,22 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
}
}
}

/// Checks whether calling `func_did` needs an `unsafe` context or not, i.e. whether
/// the called function has target features the calling function hasn't.
fn check_target_features(&mut self, func_did: DefId) {
let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
let self_features = &self.tcx.codegen_fn_attrs(self.body_did).target_features;

// Is `callee_features` a subset of `calling_features`?
if !callee_features.iter().all(|feature| self_features.contains(feature)) {
self.require_unsafe(
"call to function with `#[target_feature]`",
"can only be called if the required target features are available",
UnsafetyViolationKind::GeneralAndConstFn,
)
}
}
}

pub(crate) fn provide(providers: &mut Providers<'_>) {
Expand Down Expand Up @@ -502,7 +525,8 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: LocalDefId) -> UnsafetyCheckRe
}
hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => (true, false),
};
let mut checker = UnsafetyChecker::new(const_context, min_const_fn, body, tcx, param_env);
let mut checker =
UnsafetyChecker::new(const_context, min_const_fn, body, def_id, tcx, param_env);
checker.visit_body(&body);

check_unused_unsafe(tcx, def_id, &checker.used_unsafe, &mut checker.inherited_blocks);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,7 @@ symbols! {
suggestion,
sync_trait,
target_feature,
target_feature_11,
target_has_atomic,
target_has_atomic_load_store,
target_thread_local,
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,12 +691,22 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
debug!("coerce_from_fn_item(a={:?}, b={:?})", a, b);

match b.kind {
ty::FnPtr(_) => {
ty::FnPtr(b_sig) => {
let a_sig = a.fn_sig(self.tcx);
// Intrinsics are not coercible to function pointers
if a_sig.abi() == Abi::RustIntrinsic || a_sig.abi() == Abi::PlatformIntrinsic {
return Err(TypeError::IntrinsicCast);
}

// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).
if let ty::FnDef(def_id, _) = a.kind {
if b_sig.unsafety() == hir::Unsafety::Normal
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
{
return Err(TypeError::TargetFeatureCast(def_id));
}
}

let InferOk { value: a_sig, mut obligations } =
self.normalize_associated_types_in_as_infer_ok(self.cause.span, &a_sig);

Expand Down
51 changes: 43 additions & 8 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::weak_lang_items;
use rustc_hir::{GenericParamKind, Node, Unsafety};
use rustc_hir::{GenericParamKind, Node};
use rustc_middle::hir::map::blocks::FnLikeNode;
use rustc_middle::hir::map::Map;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
Expand Down Expand Up @@ -2404,13 +2404,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
codegen_fn_attrs.export_name = Some(s);
}
} else if attr.check_name(sym::target_feature) {
if tcx.is_closure(id) || tcx.fn_sig(id).unsafety() == Unsafety::Normal {
let msg = "`#[target_feature(..)]` can only be applied to `unsafe` functions";
tcx.sess
.struct_span_err(attr.span, msg)
.span_label(attr.span, "can only be applied to `unsafe` functions")
.span_label(tcx.def_span(id), "not an `unsafe` function")
.emit();
if !tcx.features().target_feature_11 {
check_target_feature_safe_fn(tcx, id, attr.span);
} else if let Some(local_id) = id.as_local() {
if tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
check_target_feature_trait_unsafe(tcx, local_id, attr.span);
}
}
from_target_feature(tcx, id, attr, &whitelist, &mut codegen_fn_attrs.target_features);
} else if attr.check_name(sym::linkage) {
Expand Down Expand Up @@ -2657,3 +2656,39 @@ fn check_link_name_xor_ordinal(
tcx.sess.err(msg);
}
}

/// Checks the function annotated with `#[target_feature]` is unsafe,
/// reporting an error if it isn't.
fn check_target_feature_safe_fn(tcx: TyCtxt<'_>, id: DefId, attr_span: Span) {
if tcx.is_closure(id) || tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
let mut err = feature_err(
&tcx.sess.parse_sess,
sym::target_feature_11,
attr_span,
"`#[target_feature(..)]` can only be applied to `unsafe` functions",
);
err.span_label(tcx.def_span(id), "not an `unsafe` function");
err.emit();
}
}

/// Checks the function annotated with `#[target_feature]` is not a safe
/// trait method implementation, reporting an error if it is.
fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, attr_span: Span) {
let hir_id = tcx.hir().as_local_hir_id(id);
let node = tcx.hir().get(hir_id);
if let Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }) = node {
let parent_id = tcx.hir().get_parent_item(hir_id);
let parent_item = tcx.hir().expect_item(parent_id);
if let hir::ItemKind::Impl { of_trait: Some(_), .. } = parent_item.kind {
tcx.sess
.struct_span_err(
attr_span,
"`#[target_feature(..)]` cannot be applied to safe trait method",
)
.span_label(attr_span, "cannot be applied to safe trait method")
.span_label(tcx.def_span(id), "not an `unsafe` function")
.emit();
}
}
}
10 changes: 7 additions & 3 deletions src/test/ui/macros/issue-68060.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
error: `#[target_feature(..)]` can only be applied to `unsafe` functions
error[E0658]: `#[target_feature(..)]` can only be applied to `unsafe` functions
--> $DIR/issue-68060.rs:6:13
|
LL | #[target_feature(enable = "")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can only be applied to `unsafe` functions
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | |_| (),
| ------ not an `unsafe` function
|
= note: see issue #69098 <https://github.com/rust-lang/rust/issues/69098> for more information
= help: add `#![feature(target_feature_11)]` to the crate attributes to enable

error: the feature named `` is not valid for this target
--> $DIR/issue-68060.rs:6:30
Expand All @@ -21,4 +24,5 @@ LL | #[track_caller]

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0737`.
Some errors have detailed explanations: E0658, E0737.
For more information about an error, try `rustc --explain E0658`.
50 changes: 50 additions & 0 deletions src/test/ui/rfcs/rfc-2396-target_feature-11/check-pass.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Tests the new rules added by RFC 2396, including:
// - applying `#[target_feature]` to safe functions is allowed
// - calling functions with `#[target_feature]` is allowed in
// functions which have (at least) the same features
// - calling functions with `#[target_feature]` is allowed in
// unsafe contexts
// - functions with `#[target_feature]` can coerce to unsafe fn pointers

// check-pass
// only-x86_64

#![feature(target_feature_11)]

#[target_feature(enable = "sse2")]
const fn sse2() {}

#[cfg(target_feature = "sse2")]
const SSE2_ONLY: () = unsafe {
sse2();
};

#[target_feature(enable = "sse2")]
fn also_sse2() {
sse2();
}

#[target_feature(enable = "sse2")]
#[target_feature(enable = "avx")]
fn sse2_and_avx() {
sse2();
}

struct Foo;

impl Foo {
#[target_feature(enable = "sse2")]
fn sse2(&self) {
sse2();
}
}

fn main() {
if cfg!(target_feature = "sse2") {
unsafe {
sse2();
Foo.sse2();
}
}
let sse2_ptr: unsafe fn() = sse2;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// only-x86_64

#[target_feature(enable = "sse2")] //~ ERROR can only be applied to `unsafe` functions
fn foo() {}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0658]: `#[target_feature(..)]` can only be applied to `unsafe` functions
--> $DIR/feature-gate-target_feature_11.rs:3:1
|
LL | #[target_feature(enable = "sse2")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | fn foo() {}
| ----------- not an `unsafe` function
|
= note: see issue #69098 <https://github.com/rust-lang/rust/issues/69098> for more information
= help: add `#![feature(target_feature_11)]` to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
10 changes: 10 additions & 0 deletions src/test/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// only-x86_64

#![feature(target_feature_11)]

#[target_feature(enable = "sse2")]
fn foo() {}

fn main() {
let foo: fn() = foo; //~ ERROR mismatched types
}
18 changes: 18 additions & 0 deletions src/test/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0308]: mismatched types
--> $DIR/fn-ptr.rs:9:21
|
LL | #[target_feature(enable = "sse2")]
| ---------------------------------- `#[target_feature]` added here
...
LL | let foo: fn() = foo;
| ---- ^^^ cannot coerce functions with `#[target_feature]` to safe function pointers
| |
| expected due to this
|
= note: expected fn pointer `fn()`
found fn item `fn() {foo}`
= note: functions with `#[target_feature]` can only be coerced to `unsafe` function pointers

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
Loading

0 comments on commit f05a524

Please sign in to comment.