Skip to content

Commit

Permalink
Rollup merge of rust-lang#80707 - oli-obk:stability_hole_const_intrin…
Browse files Browse the repository at this point in the history
…sics, r=RalfJung

Stability oddity with const intrinsics

cc `@RalfJung`

In rust-lang#80699 (comment) `@usbalbin` realized we accepted some intrinsics as `const` without a `#[rustc_const_(un)stable]` attribute. I did some digging, and that example works because intrinsics inherit their stability from their parents... including `#[rustc_const_(un)stable]` attributes. While we may want to fix that (not sure, wasn't there just a MCPed PR that caused this on purpose?), we definitely want tests for it, thus this PR adding tests and some fun tracing statements.
  • Loading branch information
m-ou-se committed Jan 18, 2021
2 parents 5e91c4e + 5bac1c9 commit 6f0cfce
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 18 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl NonConstOp for FnCallIndirect {

/// A function call where the callee is not marked as `const`.
#[derive(Debug)]
pub struct FnCallNonConst(pub DefId);
pub struct FnCallNonConst;
impl NonConstOp for FnCallNonConst {
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
struct_span_err!(
Expand Down
24 changes: 17 additions & 7 deletions compiler/rustc_mir/src/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,10 +789,10 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
}
}

#[instrument(skip(self))]
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
use rustc_target::spec::abi::Abi::RustIntrinsic;

trace!("visit_terminator: terminator={:?} location={:?}", terminator, location);
self.super_terminator(terminator, location);

match &terminator.kind {
Expand All @@ -816,8 +816,9 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {

// Attempting to call a trait method?
if let Some(trait_id) = tcx.trait_of_item(callee) {
trace!("attempting to call a trait method");
if !self.tcx.features().const_trait_impl {
self.check_op(ops::FnCallNonConst(callee));
self.check_op(ops::FnCallNonConst);
return;
}

Expand Down Expand Up @@ -871,25 +872,26 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
return;
}

let is_intrinsic = tcx.fn_sig(callee).abi() == RustIntrinsic;

// HACK: This is to "unstabilize" the `transmute` intrinsic
// within const fns. `transmute` is allowed in all other const contexts.
// This won't really scale to more intrinsics or functions. Let's allow const
// transmutes in const fn before we add more hacks to this.
if tcx.fn_sig(callee).abi() == RustIntrinsic
&& tcx.item_name(callee) == sym::transmute
{
if is_intrinsic && tcx.item_name(callee) == sym::transmute {
self.check_op(ops::Transmute);
return;
}

if !tcx.is_const_fn_raw(callee) {
self.check_op(ops::FnCallNonConst(callee));
self.check_op(ops::FnCallNonConst);
return;
}

// If the `const fn` we are trying to call is not const-stable, ensure that we have
// the proper feature gate enabled.
if let Some(gate) = is_unstable_const_fn(tcx, callee) {
trace!(?gate, "calling unstable const fn");
if self.span.allows_unstable(gate) {
return;
}
Expand All @@ -904,12 +906,14 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
// If this crate is not using stability attributes, or the caller is not claiming to be a
// stable `const fn`, that is all that is required.
if !self.ccx.is_const_stable_const_fn() {
trace!("crate not using stability attributes or caller not stably const");
return;
}

// Otherwise, we are something const-stable calling a const-unstable fn.

if super::rustc_allow_const_fn_unstable(tcx, caller, gate) {
trace!("rustc_allow_const_fn_unstable gate active");
return;
}

Expand All @@ -923,10 +927,16 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
let callee_is_unstable_unmarked = tcx.lookup_const_stability(callee).is_none()
&& tcx.lookup_stability(callee).map_or(false, |s| s.level.is_unstable());
if callee_is_unstable_unmarked {
if self.ccx.is_const_stable_const_fn() {
trace!("callee_is_unstable_unmarked");
// We do not use `const` modifiers for intrinsic "functions", as intrinsics are
// `extern` funtions, and these have no way to get marked `const`. So instead we
// use `rustc_const_(un)stable` attributes to mean that the intrinsic is `const`
if self.ccx.is_const_stable_const_fn() || is_intrinsic {
self.check_op(ops::FnCallUnstable(callee, None));
return;
}
}
trace!("permitting call");
}

// Forbid all `Drop` terminators unless the place being dropped is a local with no
Expand Down
70 changes: 60 additions & 10 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,21 @@ impl InheritDeprecation {
}
}

/// Whether to inherit const stability flags for nested items. In most cases, we do not want to
/// inherit const stability: just because an enclosing `fn` is const-stable does not mean
/// all `extern` imports declared in it should be const-stable! However, trait methods
/// inherit const stability attributes from their parent and do not have their own.
enum InheritConstStability {
Yes,
No,
}

impl InheritConstStability {
fn yes(&self) -> bool {
matches!(self, InheritConstStability::Yes)
}
}

// A private tree-walker for producing an Index.
struct Annotator<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
Expand All @@ -75,6 +90,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
item_sp: Span,
kind: AnnotationKind,
inherit_deprecation: InheritDeprecation,
inherit_const_stability: InheritConstStability,
visit_children: F,
) where
F: FnOnce(&mut Self),
Expand Down Expand Up @@ -140,6 +156,8 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
const_stab
});

// `impl const Trait for Type` items forward their const stability to their
// immediate children.
if const_stab.is_none() {
debug!("annotate: const_stab not found, parent = {:?}", self.parent_const_stab);
if let Some(parent) = self.parent_const_stab {
Expand Down Expand Up @@ -228,7 +246,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
self.recurse_with_stability_attrs(
depr.map(|(d, _)| DeprecationEntry::local(d, hir_id)),
stab,
const_stab,
if inherit_const_stability.yes() { const_stab } else { None },
visit_children,
);
}
Expand Down Expand Up @@ -325,6 +343,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
fn visit_item(&mut self, i: &'tcx Item<'tcx>) {
let orig_in_trait_impl = self.in_trait_impl;
let mut kind = AnnotationKind::Required;
let mut const_stab_inherit = InheritConstStability::No;
match i.kind {
// Inherent impls and foreign modules serve only as containers for other items,
// they don't have their own stability. They still can be annotated as unstable
Expand All @@ -338,6 +357,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }) => {
self.in_trait_impl = true;
kind = AnnotationKind::DeprecationProhibited;
const_stab_inherit = InheritConstStability::Yes;
}
hir::ItemKind::Struct(ref sd, _) => {
if let Some(ctor_hir_id) = sd.ctor_hir_id() {
Expand All @@ -347,16 +367,23 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
i.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|_| {},
)
}
}
_ => {}
}

self.annotate(i.hir_id, &i.attrs, i.span, kind, InheritDeprecation::Yes, |v| {
intravisit::walk_item(v, i)
});
self.annotate(
i.hir_id,
&i.attrs,
i.span,
kind,
InheritDeprecation::Yes,
const_stab_inherit,
|v| intravisit::walk_item(v, i),
);
self.in_trait_impl = orig_in_trait_impl;
}

Expand All @@ -367,6 +394,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
ti.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| {
intravisit::walk_trait_item(v, ti);
},
Expand All @@ -376,9 +404,17 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
fn visit_impl_item(&mut self, ii: &'tcx hir::ImplItem<'tcx>) {
let kind =
if self.in_trait_impl { AnnotationKind::Prohibited } else { AnnotationKind::Required };
self.annotate(ii.hir_id, &ii.attrs, ii.span, kind, InheritDeprecation::Yes, |v| {
intravisit::walk_impl_item(v, ii);
});
self.annotate(
ii.hir_id,
&ii.attrs,
ii.span,
kind,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| {
intravisit::walk_impl_item(v, ii);
},
);
}

fn visit_variant(&mut self, var: &'tcx Variant<'tcx>, g: &'tcx Generics<'tcx>, item_id: HirId) {
Expand All @@ -388,6 +424,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
var.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| {
if let Some(ctor_hir_id) = var.data.ctor_hir_id() {
v.annotate(
Expand All @@ -396,6 +433,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
var.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|_| {},
);
}
Expand All @@ -412,6 +450,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
s.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| {
intravisit::walk_struct_field(v, s);
},
Expand All @@ -425,6 +464,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
i.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| {
intravisit::walk_foreign_item(v, i);
},
Expand All @@ -438,6 +478,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
md.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|_| {},
);
}
Expand All @@ -451,9 +492,17 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
_ => AnnotationKind::Prohibited,
};

self.annotate(p.hir_id, &p.attrs, p.span, kind, InheritDeprecation::No, |v| {
intravisit::walk_generic_param(v, p);
});
self.annotate(
p.hir_id,
&p.attrs,
p.span,
kind,
InheritDeprecation::No,
InheritConstStability::No,
|v| {
intravisit::walk_generic_param(v, p);
},
);
}
}

Expand Down Expand Up @@ -618,6 +667,7 @@ fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
krate.item.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
InheritConstStability::No,
|v| intravisit::walk_crate(v, krate),
);
}
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1842,6 +1842,7 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) -
#[inline]
pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) {
extern "rust-intrinsic" {
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "80697")]
fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
}

Expand Down Expand Up @@ -1926,6 +1927,7 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
#[inline]
pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
extern "rust-intrinsic" {
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "80697")]
fn copy<T>(src: *const T, dst: *mut T, count: usize);
}

Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/consts/intrinsic_without_const_stab.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(intrinsics, staged_api, const_intrinsic_copy)]
#![stable(feature = "core", since = "1.6.0")]

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "80697")]
#[inline]
pub const unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
// Const stability attributes are not inherited from parent items.
extern "rust-intrinsic" {
fn copy<T>(src: *const T, dst: *mut T, count: usize);
}

unsafe { copy(src, dst, count) }
//~^ ERROR calls in constant functions are limited to constant functions
}

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/consts/intrinsic_without_const_stab.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0015]: calls in constant functions are limited to constant functions, tuple structs and tuple variants
--> $DIR/intrinsic_without_const_stab.rs:13:14
|
LL | unsafe { copy(src, dst, count) }
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0015`.
15 changes: 15 additions & 0 deletions src/test/ui/consts/intrinsic_without_const_stab_fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(intrinsics, staged_api, const_intrinsic_copy)]
#![stable(feature = "core", since = "1.6.0")]

extern "rust-intrinsic" {
fn copy<T>(src: *const T, dst: *mut T, count: usize);
}

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "80697")]
#[inline]
pub const unsafe fn stuff<T>(src: *const T, dst: *mut T, count: usize) {
unsafe { copy(src, dst, count) } //~ ERROR calls in constant functions are limited
}

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/consts/intrinsic_without_const_stab_fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0015]: calls in constant functions are limited to constant functions, tuple structs and tuple variants
--> $DIR/intrinsic_without_const_stab_fail.rs:12:14
|
LL | unsafe { copy(src, dst, count) }
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

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

0 comments on commit 6f0cfce

Please sign in to comment.