Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RFC 2396: #[target_feature] 1.1 #69274

Merged
merged 4 commits into from
May 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
Copy link
Contributor

@hanna-kruppe hanna-kruppe May 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not even a nit, just curious: what does this affect, and why is TargetFeatureCast different from the similar-in-spirit IntrinsicCast here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there is no particular reason. This is my first major non-minor contribution to the compiler, so I basically searched for things similar to what I tried to implement. I wonder if we could merge the two though, since I use the distinction between the two to have an adapted error message.


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 @@ -688,12 +688,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 @@ -2413,13 +2413,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();
LeSeulArtichaut marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2666,3 +2665,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
LeSeulArtichaut marked this conversation as resolved.
Show resolved Hide resolved
// 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