From fca0666e01430d7354d6d85c5cc047c333dca214 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 3 Feb 2020 10:19:33 -0800 Subject: [PATCH 1/5] Assign structured errors to `min_const_fn` checks --- .../transform/check_consts/mod.rs | 7 + .../transform/check_consts/ops.rs | 260 +++++++++++++---- .../transform/check_consts/validation.rs | 265 +++++++++++++----- src/librustc_mir/transform/mod.rs | 4 +- src/librustc_passes/check_const.rs | 1 - 5 files changed, 407 insertions(+), 130 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs index b7383663932a4..41c088153c80f 100644 --- a/src/librustc_mir/transform/check_consts/mod.rs +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -111,3 +111,10 @@ impl fmt::Display for ConstKind { pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { Some(def_id) == tcx.lang_items().panic_fn() || Some(def_id) == tcx.lang_items().begin_panic_fn() } + +/// Returns `true` if the constness of this item is not stabilized, either because it is declared +/// with `#![rustc_const_unstable]` or because the item itself is unstable. +fn is_const_unstable(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { + tcx.lookup_const_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) + || tcx.lookup_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) +} diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs index 3263905eadb81..e0f78f548972d 100644 --- a/src/librustc_mir/transform/check_consts/ops.rs +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -1,23 +1,25 @@ //! Concrete error types for all operations which may be invalid in a certain const context. +use rustc::mir; use rustc::session::config::nightly_options; use rustc::session::parse::feature_err; -use rustc::ty::TyCtxt; +use rustc::ty::Ty; use rustc_errors::struct_span_err; use rustc_hir::def_id::DefId; +use rustc_hir::Constness; use rustc_span::symbol::sym; use rustc_span::{Span, Symbol}; -use super::{ConstKind, Item}; +use super::{is_const_unstable, ConstKind, Item}; /// An operation that is not *always* allowed in a const context. pub trait NonConstOp: std::fmt::Debug { /// Whether this operation can be evaluated by miri. const IS_SUPPORTED_IN_MIRI: bool = true; - /// Returns a boolean indicating whether the feature gate that would allow this operation is - /// enabled, or `None` if such a feature gate does not exist. - fn feature_gate(_tcx: TyCtxt<'tcx>) -> Option { + /// Returns the `Symbol` for the feature gate that would allow this operation, or `None` if + /// such a feature gate does not exist. + fn feature_gate() -> Option { None } @@ -26,7 +28,7 @@ pub trait NonConstOp: std::fmt::Debug { /// This check should assume that we are not in a non-const `fn`, where all operations are /// legal. fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { - Self::feature_gate(item.tcx).unwrap_or(false) + Self::feature_gate().map_or(false, |gate| feature_allowed(item, gate)) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -51,12 +53,48 @@ pub trait NonConstOp: std::fmt::Debug { } } +#[derive(Debug)] +pub struct Abort; +impl NonConstOp for Abort {} + +#[derive(Debug)] +pub enum ArithmeticOp { + Binary(mir::BinOp), + Unary(mir::UnOp), +} + +impl From for ArithmeticOp { + fn from(op: mir::BinOp) -> Self { + ArithmeticOp::Binary(op) + } +} + +impl From for ArithmeticOp { + fn from(op: mir::UnOp) -> Self { + ArithmeticOp::Unary(op) + } +} + +#[derive(Debug)] +pub struct Arithmetic<'a>(pub ArithmeticOp, pub Ty<'a>); +impl NonConstOp for Arithmetic<'_> { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.const_kind() != ConstKind::ConstFn || !min_const_fn_checks_enabled(item) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let msg = + format!("operations on `{:?}` are not allowed in a {}", self.1, item.const_kind()); + feature_err(&item.tcx.sess.parse_sess, sym::const_fn, span, &msg).emit() + } +} + /// A `Downcast` projection. #[derive(Debug)] pub struct Downcast; impl NonConstOp for Downcast { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_if_match) + fn feature_gate() -> Option { + Some(sym::const_if_match) } } @@ -65,11 +103,7 @@ impl NonConstOp for Downcast { pub struct FnCallIndirect; impl NonConstOp for FnCallIndirect { fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = item - .tcx - .sess - .struct_span_err(span, &format!("function pointers are not allowed in const fn")); - err.emit(); + item.tcx.sess.struct_span_err(span, "indirect function calls are not supported").emit() } } @@ -120,6 +154,10 @@ impl NonConstOp for FnCallUnstable { } } +#[derive(Debug)] +pub struct Generator; +impl NonConstOp for Generator {} + #[derive(Debug)] pub struct HeapAllocation; impl NonConstOp for HeapAllocation { @@ -149,8 +187,8 @@ impl NonConstOp for HeapAllocation { #[derive(Debug)] pub struct IfOrMatch; impl NonConstOp for IfOrMatch { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_if_match) + fn feature_gate() -> Option { + Some(sym::const_if_match) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -159,6 +197,10 @@ impl NonConstOp for IfOrMatch { } } +#[derive(Debug)] +pub struct InlineAsm; +impl NonConstOp for InlineAsm {} + #[derive(Debug)] pub struct LiveDrop; impl NonConstOp for LiveDrop { @@ -177,8 +219,8 @@ impl NonConstOp for LiveDrop { #[derive(Debug)] pub struct Loop; impl NonConstOp for Loop { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_loop) + fn feature_gate() -> Option { + Some(sym::const_loop) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -205,8 +247,8 @@ impl NonConstOp for CellBorrow { #[derive(Debug)] pub struct MutBorrow; impl NonConstOp for MutBorrow { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_mut_refs) + fn feature_gate() -> Option { + Some(sym::const_mut_refs) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -214,11 +256,7 @@ impl NonConstOp for MutBorrow { &item.tcx.sess.parse_sess, sym::const_mut_refs, span, - &format!( - "references in {}s may only refer \ - to immutable values", - item.const_kind() - ), + &format!("mutable references in a {} are unstable", item.const_kind()), ); err.span_label(span, format!("{}s require immutable values", item.const_kind())); if item.tcx.sess.teach(&err.get_code().unwrap()) { @@ -237,37 +275,19 @@ impl NonConstOp for MutBorrow { } } -#[derive(Debug)] -pub struct MutAddressOf; -impl NonConstOp for MutAddressOf { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_mut_refs) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - feature_err( - &item.tcx.sess.parse_sess, - sym::const_mut_refs, - span, - &format!("`&raw mut` is not allowed in {}s", item.const_kind()), - ) - .emit(); - } -} - #[derive(Debug)] pub struct MutDeref; impl NonConstOp for MutDeref { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_mut_refs) + fn feature_gate() -> Option { + Some(sym::const_mut_refs) } } #[derive(Debug)] pub struct Panic; impl NonConstOp for Panic { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_panic) + fn feature_gate() -> Option { + Some(sym::const_panic) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -284,8 +304,8 @@ impl NonConstOp for Panic { #[derive(Debug)] pub struct RawPtrComparison; impl NonConstOp for RawPtrComparison { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_compare_raw_pointers) + fn feature_gate() -> Option { + Some(sym::const_compare_raw_pointers) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -302,8 +322,8 @@ impl NonConstOp for RawPtrComparison { #[derive(Debug)] pub struct RawPtrDeref; impl NonConstOp for RawPtrDeref { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_raw_ptr_deref) + fn feature_gate() -> Option { + Some(sym::const_raw_ptr_deref) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -311,7 +331,7 @@ impl NonConstOp for RawPtrDeref { &item.tcx.sess.parse_sess, sym::const_raw_ptr_deref, span, - &format!("dereferencing raw pointers in {}s is unstable", item.const_kind(),), + &format!("dereferencing raw pointers in {}s is unstable", item.const_kind()), ) .emit(); } @@ -320,8 +340,8 @@ impl NonConstOp for RawPtrDeref { #[derive(Debug)] pub struct RawPtrToIntCast; impl NonConstOp for RawPtrToIntCast { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_raw_ptr_to_usize_cast) + fn feature_gate() -> Option { + Some(sym::const_raw_ptr_to_usize_cast) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -329,12 +349,98 @@ impl NonConstOp for RawPtrToIntCast { &item.tcx.sess.parse_sess, sym::const_raw_ptr_to_usize_cast, span, - &format!("casting pointers to integers in {}s is unstable", item.const_kind(),), + &format!("casting pointers to integers in {}s is unstable", item.const_kind()), ) .emit(); } } +#[derive(Debug)] +pub struct FnPtr; +impl NonConstOp for FnPtr { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.const_kind() != ConstKind::ConstFn + || !min_const_fn_checks_enabled(item) + || item.tcx.has_attr(item.def_id, sym::rustc_allow_const_fn_ptr) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + feature_err( + &item.tcx.sess.parse_sess, + sym::const_fn, + span, + "function pointers in const fn are unstable", + ) + .emit() + } +} + +/// See [#64992]. +/// +/// [#64992]: https://github.com/rust-lang/rust/issues/64992 +#[derive(Debug)] +pub struct UnsizingCast; +impl NonConstOp for UnsizingCast { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.const_kind() != ConstKind::ConstFn || !min_const_fn_checks_enabled(item) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + feature_err( + &item.tcx.sess.parse_sess, + sym::const_fn, + span, + "unsizing casts are not allowed in const fn", + ) + .emit() + } +} + +#[derive(Debug)] +pub struct ImplTrait; +impl NonConstOp for ImplTrait { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.const_kind() != ConstKind::ConstFn || !min_const_fn_checks_enabled(item) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + feature_err( + &item.tcx.sess.parse_sess, + sym::const_fn, + span, + "`impl Trait` in const fn is unstable", + ) + .emit() + } +} + +#[derive(Debug)] +pub struct TraitBound(pub Constness); +impl NonConstOp for TraitBound { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + if item.const_kind() != ConstKind::ConstFn || !min_const_fn_checks_enabled(item) { + return true; + } + + // Allow `T: ?const Trait` + if self.0 == Constness::NotConst { + feature_allowed(item, sym::const_trait_bound_opt_out) + } else { + false + } + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + feature_err( + &item.tcx.sess.parse_sess, + sym::const_fn, + span, + "trait bounds other than `Sized` on const fn parameters are unstable", + ) + .emit() + } +} + /// An access to a (non-thread-local) `static`. #[derive(Debug)] pub struct StaticAccess; @@ -388,11 +494,11 @@ pub struct UnionAccess; impl NonConstOp for UnionAccess { fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { // Union accesses are stable in all contexts except `const fn`. - item.const_kind() != ConstKind::ConstFn || Self::feature_gate(item.tcx).unwrap() + item.const_kind() != ConstKind::ConstFn || feature_allowed(item, sym::const_fn_union) } - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_fn_union) + fn feature_gate() -> Option { + Some(sym::const_fn_union) } fn emit_error(&self, item: &Item<'_, '_>, span: Span) { @@ -405,3 +511,43 @@ impl NonConstOp for UnionAccess { .emit(); } } + +/// Returns `true` if the feature with the given gate is allowed within this const context. +fn feature_allowed(item: &Item<'_, '_>, feature_gate: Symbol) -> bool { + let Item { tcx, def_id, .. } = *item; + + // All features require that the corresponding gate be enabled, + // even if the function has `#[allow_internal_unstable(the_gate)]`. + if !tcx.features().enabled(feature_gate) { + return false; + } + + // If this crate is not using stability attributes, or this function is not claiming to be a + // stable `const fn`, that is all that is required. + if !tcx.features().staged_api { + return true; + } + + if is_const_unstable(item.tcx, item.def_id) { + return true; + } + + // However, we cannot allow stable `const fn`s to use unstable features without an explicit + // opt-in via `allow_internal_unstable`. + rustc_attr::allow_internal_unstable(&tcx.get_attrs(def_id), &tcx.sess.diagnostic()) + .map_or(false, |mut features| features.any(|name| name == feature_gate)) +} + +fn min_const_fn_checks_enabled(item: &Item<'_, '_>) -> bool { + assert_eq!(item.const_kind(), ConstKind::ConstFn); + + if item.tcx.features().staged_api { + // All functions except for unstable ones need to pass the min const fn checks. This + // includes private functions that are not marked unstable. + !is_const_unstable(item.tcx, item.def_id) + } else { + // Crates that are not using stability attributes can use `#![feature(const_fn)]` to opt out of + // the min_const_fn checks. + !item.tcx.features().const_fn + } +} diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 35107a31aa122..620d1ee8f3d11 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -5,14 +5,12 @@ use rustc::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, use rustc::mir::*; use rustc::traits::{self, TraitEngine}; use rustc::ty::cast::CastTy; -use rustc::ty::{self, TyCtxt}; -use rustc_errors::struct_span_err; -use rustc_hir::{def_id::DefId, HirId}; +use rustc::ty::{self, adjustment::PointerCast, Predicate, Ty, TyCtxt}; +use rustc_hir::{self as hir, def_id::DefId, HirId}; use rustc_index::bit_set::BitSet; use rustc_span::symbol::sym; use rustc_span::Span; -use std::borrow::Cow; use std::ops::Deref; use self::old_dataflow::IndirectlyMutableLocals; @@ -20,7 +18,7 @@ use super::ops::{self, NonConstOp}; use super::qualifs::{self, HasMutInterior, NeedsDrop}; use super::resolver::FlowSensitiveAnalysis; use super::{is_lang_panic_fn, ConstKind, Item, Qualif}; -use crate::const_eval::{is_const_fn, is_unstable_const_fn}; +use crate::const_eval::{is_const_fn, is_min_const_fn, is_unstable_const_fn}; use crate::dataflow::{self as old_dataflow, generic as dataflow}; pub type IndirectlyMutableResults<'mir, 'tcx> = @@ -155,20 +153,24 @@ impl Validator<'a, 'mir, 'tcx> { Validator { span: item.body.span, item, qualifs } } - pub fn check_body(&mut self) { + pub fn check_item(&mut self) { let Item { tcx, body, def_id, const_kind, .. } = *self.item; - let use_min_const_fn_checks = (const_kind == Some(ConstKind::ConstFn) - && crate::const_eval::is_min_const_fn(tcx, def_id)) - && !tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you; + // The local type and predicate checks are remnants from the old `min_const_fn` + // pass, so they only run on `const fn`s. + if const_kind == Some(ConstKind::ConstFn) { + self.check_item_predicates(); - if use_min_const_fn_checks { - // Enforce `min_const_fn` for stable `const fn`s. - use crate::transform::qualify_min_const_fn::is_min_const_fn; - if let Err((span, err)) = is_min_const_fn(tcx, def_id, &body) { - error_min_const_fn_violation(tcx, span, err); - return; + for local in &body.local_decls { + self.span = local.source_info.span; + self.check_local_or_return_ty(local.ty); } + + // impl trait is gone in MIR, so check the return type of a const fn by its signature + // instead of the type of the return place. + self.span = body.local_decls[RETURN_PLACE].source_info.span; + let return_ty = tcx.fn_sig(def_id).output(); + self.check_local_or_return_ty(return_ty.skip_binder()); } check_short_circuiting_in_const_local(self.item); @@ -201,7 +203,7 @@ impl Validator<'a, 'mir, 'tcx> { where O: NonConstOp, { - trace!("check_op: op={:?}", op); + debug!("check_op: op={:?}", op); if op.is_allowed_in_item(self) { return; @@ -209,8 +211,7 @@ impl Validator<'a, 'mir, 'tcx> { // If an operation is supported in miri (and is not already controlled by a feature gate) it // can be turned on with `-Zunleash-the-miri-inside-of-you`. - let is_unleashable = O::IS_SUPPORTED_IN_MIRI && O::feature_gate(self.tcx).is_none(); - + let is_unleashable = O::IS_SUPPORTED_IN_MIRI && O::feature_gate().is_none(); if is_unleashable && self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { self.tcx.sess.span_warn(span, "skipping const checks"); return; @@ -233,6 +234,81 @@ impl Validator<'a, 'mir, 'tcx> { self.check_op_spanned(ops::StaticAccess, span) } } + + fn check_local_or_return_ty(&mut self, ty: Ty<'tcx>) { + for ty in ty.walk() { + match ty.kind { + ty::Ref(_, _, hir::Mutability::Mut) => self.check_op(ops::MutBorrow), + ty::Opaque(..) => self.check_op(ops::ImplTrait), + ty::FnPtr(..) => self.check_op(ops::FnPtr), + + ty::Dynamic(preds, _) => { + for pred in preds.iter() { + match pred.skip_binder() { + ty::ExistentialPredicate::AutoTrait(_) + | ty::ExistentialPredicate::Projection(_) => { + self.check_op(ops::TraitBound(hir::Constness::Const)) + } + ty::ExistentialPredicate::Trait(trait_ref) => { + if Some(trait_ref.def_id) != self.tcx.lang_items().sized_trait() { + self.check_op(ops::TraitBound(hir::Constness::Const)) + } + } + } + } + } + _ => {} + } + } + } + + fn check_item_predicates(&mut self) { + let Item { tcx, def_id, .. } = *self.item; + + let mut current = def_id; + loop { + let predicates = tcx.predicates_of(current); + for (predicate, _) in predicates.predicates { + match predicate { + Predicate::RegionOutlives(_) + | Predicate::TypeOutlives(_) + | Predicate::WellFormed(_) + | Predicate::Projection(_) + | Predicate::ConstEvaluatable(..) => continue, + Predicate::ObjectSafe(_) => { + bug!("object safe predicate on function: {:#?}", predicate) + } + Predicate::ClosureKind(..) => { + bug!("closure kind predicate on function: {:#?}", predicate) + } + Predicate::Subtype(_) => { + bug!("subtype predicate on function: {:#?}", predicate) + } + Predicate::Trait(pred, constness) => { + if Some(pred.def_id()) == tcx.lang_items().sized_trait() { + continue; + } + match pred.skip_binder().self_ty().kind { + ty::Param(ref p) => { + let generics = tcx.generics_of(current); + let def = generics.type_param(p, tcx); + let span = tcx.def_span(def.def_id); + + self.check_op_spanned(ops::TraitBound(*constness), span); + } + // other kinds of bounds are either tautologies + // or cause errors in other passes + _ => continue, + } + } + } + } + match predicates.parent { + Some(parent) => current = parent, + None => break, + } + } + } } impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { @@ -298,17 +374,15 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { match *rvalue { Rvalue::Use(_) | Rvalue::Repeat(..) - | Rvalue::UnaryOp(UnOp::Neg, _) - | Rvalue::UnaryOp(UnOp::Not, _) - | Rvalue::NullaryOp(NullOp::SizeOf, _) - | Rvalue::CheckedBinaryOp(..) - | Rvalue::Cast(CastKind::Pointer(_), ..) | Rvalue::Discriminant(..) | Rvalue::Len(_) | Rvalue::Aggregate(..) => {} - Rvalue::Ref(_, kind @ BorrowKind::Mut { .. }, ref place) - | Rvalue::Ref(_, kind @ BorrowKind::Unique, ref place) => { + // `&mut` and `&raw mut` + Rvalue::AddressOf(Mutability::Mut, _) => self.check_op(ops::MutBorrow), + + Rvalue::Ref(_, BorrowKind::Mut { .. }, ref place) + | Rvalue::Ref(_, BorrowKind::Unique, ref place) => { let ty = place.ty(*self.body, self.tcx).ty; let is_allowed = match ty.kind { // Inside a `static mut`, `&mut [...]` is allowed. @@ -327,16 +401,10 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { }; if !is_allowed { - if let BorrowKind::Mut { .. } = kind { - self.check_op(ops::MutBorrow); - } else { - self.check_op(ops::CellBorrow); - } + self.check_op(ops::MutBorrow); } } - Rvalue::AddressOf(Mutability::Mut, _) => self.check_op(ops::MutAddressOf), - Rvalue::Ref(_, BorrowKind::Shared, ref place) | Rvalue::Ref(_, BorrowKind::Shallow, ref place) | Rvalue::AddressOf(Mutability::Not, ref place) => { @@ -351,6 +419,19 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } } + Rvalue::Cast(CastKind::Pointer(PointerCast::UnsafeFnPointer), ..) + | Rvalue::Cast(CastKind::Pointer(PointerCast::ClosureFnPointer(_)), ..) + | Rvalue::Cast(CastKind::Pointer(PointerCast::ReifyFnPointer), ..) => { + self.check_op(ops::FnPtr) + } + + Rvalue::Cast(CastKind::Pointer(PointerCast::MutToConstPointer), ..) + | Rvalue::Cast(CastKind::Pointer(PointerCast::ArrayToPointer), ..) => {} + + Rvalue::Cast(CastKind::Pointer(PointerCast::Unsize), ..) => { + self.check_op(ops::UnsizingCast) + } + Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => { let operand_ty = operand.ty(*self.body, self.tcx); let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast"); @@ -363,24 +444,43 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } } - Rvalue::BinaryOp(op, ref lhs, _) => { - if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(*self.body, self.tcx).kind { - assert!( - op == BinOp::Eq - || op == BinOp::Ne - || op == BinOp::Le - || op == BinOp::Lt - || op == BinOp::Ge - || op == BinOp::Gt - || op == BinOp::Offset - ); - - self.check_op(ops::RawPtrComparison); + Rvalue::NullaryOp(NullOp::SizeOf, _) => {} + Rvalue::NullaryOp(NullOp::Box, _) => self.check_op(ops::HeapAllocation), + + Rvalue::UnaryOp(op, ref place) => { + let ty = place.ty(*self.body, self.tcx); + match ty.kind { + // Operations on the following primitive types are always allowed. + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) => {} + + // All other operations are forbidden + _ => self.check_op(ops::Arithmetic(op.into(), ty)), } } - Rvalue::NullaryOp(NullOp::Box, _) => { - self.check_op(ops::HeapAllocation); + Rvalue::CheckedBinaryOp(op, ref lhs, _) | Rvalue::BinaryOp(op, ref lhs, _) => { + let ty = lhs.ty(*self.body, self.tcx); + match ty.kind { + ty::RawPtr(_) | ty::FnPtr(..) => { + assert!( + op == BinOp::Eq + || op == BinOp::Ne + || op == BinOp::Le + || op == BinOp::Lt + || op == BinOp::Ge + || op == BinOp::Gt + || op == BinOp::Offset + ); + + self.check_op(ops::RawPtrComparison); + } + + // Operations on the following primitive types are always allowed. + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) => {} + + // All other operations are forbidden + _ => self.check_op(ops::Arithmetic(op.into(), ty)), + } } } } @@ -477,14 +577,22 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { StatementKind::Assign(..) | StatementKind::SetDiscriminant { .. } => { self.super_statement(statement, location); } - StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => { - self.check_op(ops::IfOrMatch); + + StatementKind::InlineAsm { .. } => self.check_op(ops::InlineAsm), + + // Try to ensure that no `match` expressions have gotten throught the HIR const-checker. + StatementKind::FakeRead(FakeReadCause::ForMatchGuard, _) + | StatementKind::FakeRead(FakeReadCause::ForGuardBinding, _) + | StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => { + self.check_op(ops::IfOrMatch) } - // FIXME(eddyb) should these really do nothing? - StatementKind::FakeRead(..) - | StatementKind::StorageLive(_) + + // Other `FakeRead`s are allowed + StatementKind::FakeRead(FakeReadCause::ForLet, _) + | StatementKind::FakeRead(FakeReadCause::ForIndex, _) => {} + + StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - | StatementKind::InlineAsm { .. } | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Nop => {} @@ -496,10 +604,25 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { self.super_terminator_kind(kind, location); match kind { + TerminatorKind::FalseEdges { .. } + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::Goto { .. } + | TerminatorKind::Return + | TerminatorKind::Unreachable + | TerminatorKind::Resume => {} + + // Emitted when, e.g., doing integer division to detect division by zero. + TerminatorKind::Assert { .. } => {} + + TerminatorKind::SwitchInt { .. } => self.check_op(ops::IfOrMatch), + TerminatorKind::Abort => self.check_op(ops::Abort), + TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { + self.check_op(ops::Generator) + } + TerminatorKind::Call { func, .. } => { let fn_ty = func.ty(*self.body, self.tcx); - - let def_id = match fn_ty.kind { + let callee = match fn_ty.kind { ty::FnDef(def_id, _) => def_id, ty::FnPtr(_) => { @@ -512,21 +635,32 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } }; - // At this point, we are calling a function whose `DefId` is known... - if is_const_fn(self.tcx, def_id) { + let Item { tcx, def_id: caller, .. } = *self.item; + + if self.const_kind == Some(ConstKind::ConstFn) + && is_min_const_fn(tcx, caller) + && !is_min_const_fn(tcx, callee) + && is_unstable_const_fn(tcx, callee) + .map_or(true, |feature| !self.span.allows_unstable(feature)) + { + tcx.sess + .span_err(self.span, "Stable `const fn`s cannot call unstable `const fn`s"); + } + + if is_const_fn(tcx, callee) { return; } - if is_lang_panic_fn(self.tcx, def_id) { + if is_lang_panic_fn(tcx, callee) { self.check_op(ops::Panic); - } else if let Some(feature) = is_unstable_const_fn(self.tcx, def_id) { + } else if let Some(feature) = is_unstable_const_fn(tcx, callee) { // Exempt unstable const fns inside of macros with // `#[allow_internal_unstable]`. if !self.span.allows_unstable(feature) { - self.check_op(ops::FnCallUnstable(def_id, feature)); + self.check_op(ops::FnCallUnstable(callee, feature)); } } else { - self.check_op(ops::FnCallNonConst(def_id)); + self.check_op(ops::FnCallNonConst(callee)); } } @@ -557,19 +691,10 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { self.check_op_spanned(ops::LiveDrop, err_span); } } - - _ => {} } } } -fn error_min_const_fn_violation(tcx: TyCtxt<'_>, span: Span, msg: Cow<'_, str>) { - struct_span_err!(tcx.sess, span, E0723, "{}", msg) - .note("for more information, see issue https://github.com/rust-lang/rust/issues/57563") - .help("add `#![feature(const_fn)]` to the crate attributes to enable") - .emit(); -} - fn check_short_circuiting_in_const_local(item: &Item<'_, 'tcx>) { let body = item.body; diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 3c37eccc1843b..cb3441bf33138 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -29,7 +29,7 @@ pub mod inline; pub mod instcombine; pub mod no_landing_pads; pub mod promote_consts; -pub mod qualify_min_const_fn; +//pub mod qualify_min_const_fn; pub mod remove_noop_landing_pads; pub mod rustc_peek; pub mod simplify; @@ -205,7 +205,7 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> ConstQualifs { }; let mut validator = check_consts::validation::Validator::new(&item); - validator.check_body(); + validator.check_item(); // We return the qualifs in the return place for every MIR body, even though it is only used // when deciding to promote a reference to a `const` for now. diff --git a/src/librustc_passes/check_const.rs b/src/librustc_passes/check_const.rs index b178110f4f954..6aacd4d4be8eb 100644 --- a/src/librustc_passes/check_const.rs +++ b/src/librustc_passes/check_const.rs @@ -8,7 +8,6 @@ //! through, but errors for structured control flow in a `const` should be emitted here. use rustc::hir::map::Map; -use rustc::hir::Hir; use rustc::session::config::nightly_options; use rustc::session::parse::feature_err; use rustc::ty::query::Providers; From 01c33e96a74f9cad6c4985189a082f59e316a2f5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 3 Feb 2020 10:21:04 -0800 Subject: [PATCH 2/5] Remove `qualify_min_const_fn` --- src/librustc_mir/transform/mod.rs | 1 - .../transform/qualify_min_const_fn.rs | 380 ------------------ 2 files changed, 381 deletions(-) delete mode 100644 src/librustc_mir/transform/qualify_min_const_fn.rs diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index cb3441bf33138..15cd935589ed9 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -29,7 +29,6 @@ pub mod inline; pub mod instcombine; pub mod no_landing_pads; pub mod promote_consts; -//pub mod qualify_min_const_fn; pub mod remove_noop_landing_pads; pub mod rustc_peek; pub mod simplify; diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs deleted file mode 100644 index 6a68ccdddffdc..0000000000000 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ /dev/null @@ -1,380 +0,0 @@ -use rustc::mir::*; -use rustc::ty::{self, adjustment::PointerCast, Predicate, Ty, TyCtxt}; -use rustc_attr as attr; -use rustc_hir as hir; -use rustc_hir::def_id::DefId; -use rustc_span::symbol::{sym, Symbol}; -use rustc_span::Span; -use std::borrow::Cow; -use syntax::ast; - -type McfResult = Result<(), (Span, Cow<'static, str>)>; - -pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, def_id: DefId, body: &'a Body<'tcx>) -> McfResult { - let mut current = def_id; - loop { - let predicates = tcx.predicates_of(current); - for (predicate, _) in predicates.predicates { - match predicate { - Predicate::RegionOutlives(_) - | Predicate::TypeOutlives(_) - | Predicate::WellFormed(_) - | Predicate::Projection(_) - | Predicate::ConstEvaluatable(..) => continue, - Predicate::ObjectSafe(_) => { - bug!("object safe predicate on function: {:#?}", predicate) - } - Predicate::ClosureKind(..) => { - bug!("closure kind predicate on function: {:#?}", predicate) - } - Predicate::Subtype(_) => bug!("subtype predicate on function: {:#?}", predicate), - Predicate::Trait(pred, constness) => { - if Some(pred.def_id()) == tcx.lang_items().sized_trait() { - continue; - } - match pred.skip_binder().self_ty().kind { - ty::Param(ref p) => { - // Allow `T: ?const Trait` - if *constness == ast::Constness::NotConst - && feature_allowed(tcx, def_id, sym::const_trait_bound_opt_out) - { - continue; - } - - let generics = tcx.generics_of(current); - let def = generics.type_param(p, tcx); - let span = tcx.def_span(def.def_id); - return Err(( - span, - "trait bounds other than `Sized` \ - on const fn parameters are unstable" - .into(), - )); - } - // other kinds of bounds are either tautologies - // or cause errors in other passes - _ => continue, - } - } - } - } - match predicates.parent { - Some(parent) => current = parent, - None => break, - } - } - - for local in &body.local_decls { - check_ty(tcx, local.ty, local.source_info.span, def_id)?; - } - // impl trait is gone in MIR, so check the return type manually - check_ty( - tcx, - tcx.fn_sig(def_id).output().skip_binder(), - body.local_decls.iter().next().unwrap().source_info.span, - def_id, - )?; - - for bb in body.basic_blocks() { - check_terminator(tcx, body, def_id, bb.terminator())?; - for stmt in &bb.statements { - check_statement(tcx, body, def_id, stmt)?; - } - } - Ok(()) -} - -fn check_ty(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, span: Span, fn_def_id: DefId) -> McfResult { - for ty in ty.walk() { - match ty.kind { - ty::Ref(_, _, hir::Mutability::Mut) => { - if !feature_allowed(tcx, fn_def_id, sym::const_mut_refs) { - return Err((span, "mutable references in const fn are unstable".into())); - } - } - ty::Opaque(..) => return Err((span, "`impl Trait` in const fn is unstable".into())), - ty::FnPtr(..) => { - if !tcx.const_fn_is_allowed_fn_ptr(fn_def_id) { - return Err((span, "function pointers in const fn are unstable".into())); - } - } - ty::Dynamic(preds, _) => { - for pred in preds.iter() { - match pred.skip_binder() { - ty::ExistentialPredicate::AutoTrait(_) - | ty::ExistentialPredicate::Projection(_) => { - return Err(( - span, - "trait bounds other than `Sized` \ - on const fn parameters are unstable" - .into(), - )); - } - ty::ExistentialPredicate::Trait(trait_ref) => { - if Some(trait_ref.def_id) != tcx.lang_items().sized_trait() { - return Err(( - span, - "trait bounds other than `Sized` \ - on const fn parameters are unstable" - .into(), - )); - } - } - } - } - } - _ => {} - } - } - Ok(()) -} - -fn check_rvalue( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - def_id: DefId, - rvalue: &Rvalue<'tcx>, - span: Span, -) -> McfResult { - match rvalue { - Rvalue::Repeat(operand, _) | Rvalue::Use(operand) => { - check_operand(tcx, operand, span, def_id, body) - } - Rvalue::Len(place) - | Rvalue::Discriminant(place) - | Rvalue::Ref(_, _, place) - | Rvalue::AddressOf(_, place) => check_place(tcx, place, span, def_id, body), - Rvalue::Cast(CastKind::Misc, operand, cast_ty) => { - use rustc::ty::cast::CastTy; - let cast_in = CastTy::from_ty(operand.ty(body, tcx)).expect("bad input type for cast"); - let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); - match (cast_in, cast_out) { - (CastTy::Ptr(_), CastTy::Int(_)) | (CastTy::FnPtr, CastTy::Int(_)) => { - Err((span, "casting pointers to ints is unstable in const fn".into())) - } - _ => check_operand(tcx, operand, span, def_id, body), - } - } - Rvalue::Cast(CastKind::Pointer(PointerCast::MutToConstPointer), operand, _) - | Rvalue::Cast(CastKind::Pointer(PointerCast::ArrayToPointer), operand, _) => { - check_operand(tcx, operand, span, def_id, body) - } - Rvalue::Cast(CastKind::Pointer(PointerCast::UnsafeFnPointer), _, _) - | Rvalue::Cast(CastKind::Pointer(PointerCast::ClosureFnPointer(_)), _, _) - | Rvalue::Cast(CastKind::Pointer(PointerCast::ReifyFnPointer), _, _) => { - Err((span, "function pointer casts are not allowed in const fn".into())) - } - Rvalue::Cast(CastKind::Pointer(PointerCast::Unsize), _, _) => { - Err((span, "unsizing casts are not allowed in const fn".into())) - } - // binops are fine on integers - Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => { - check_operand(tcx, lhs, span, def_id, body)?; - check_operand(tcx, rhs, span, def_id, body)?; - let ty = lhs.ty(body, tcx); - if ty.is_integral() || ty.is_bool() || ty.is_char() { - Ok(()) - } else { - Err((span, "only int, `bool` and `char` operations are stable in const fn".into())) - } - } - Rvalue::NullaryOp(NullOp::SizeOf, _) => Ok(()), - Rvalue::NullaryOp(NullOp::Box, _) => { - Err((span, "heap allocations are not allowed in const fn".into())) - } - Rvalue::UnaryOp(_, operand) => { - let ty = operand.ty(body, tcx); - if ty.is_integral() || ty.is_bool() { - check_operand(tcx, operand, span, def_id, body) - } else { - Err((span, "only int and `bool` operations are stable in const fn".into())) - } - } - Rvalue::Aggregate(_, operands) => { - for operand in operands { - check_operand(tcx, operand, span, def_id, body)?; - } - Ok(()) - } - } -} - -fn check_statement( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - def_id: DefId, - statement: &Statement<'tcx>, -) -> McfResult { - let span = statement.source_info.span; - match &statement.kind { - StatementKind::Assign(box (place, rval)) => { - check_place(tcx, place, span, def_id, body)?; - check_rvalue(tcx, body, def_id, rval, span) - } - - StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) - if !feature_allowed(tcx, def_id, sym::const_if_match) => - { - Err((span, "loops and conditional expressions are not stable in const fn".into())) - } - - StatementKind::FakeRead(_, place) => check_place(tcx, place, span, def_id, body), - - // just an assignment - StatementKind::SetDiscriminant { place, .. } => check_place(tcx, place, span, def_id, body), - - StatementKind::InlineAsm { .. } => { - Err((span, "cannot use inline assembly in const fn".into())) - } - - // These are all NOPs - StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - | StatementKind::Retag { .. } - | StatementKind::AscribeUserType(..) - | StatementKind::Nop => Ok(()), - } -} - -fn check_operand( - tcx: TyCtxt<'tcx>, - operand: &Operand<'tcx>, - span: Span, - def_id: DefId, - body: &Body<'tcx>, -) -> McfResult { - match operand { - Operand::Move(place) | Operand::Copy(place) => check_place(tcx, place, span, def_id, body), - Operand::Constant(c) => match c.check_static_ptr(tcx) { - Some(_) => Err((span, "cannot access `static` items in const fn".into())), - None => Ok(()), - }, - } -} - -fn check_place( - tcx: TyCtxt<'tcx>, - place: &Place<'tcx>, - span: Span, - def_id: DefId, - body: &Body<'tcx>, -) -> McfResult { - let mut cursor = place.projection.as_ref(); - while let &[ref proj_base @ .., elem] = cursor { - cursor = proj_base; - match elem { - ProjectionElem::Downcast(..) if !feature_allowed(tcx, def_id, sym::const_if_match) => { - return Err((span, "`match` or `if let` in `const fn` is unstable".into())); - } - ProjectionElem::Downcast(_symbol, _variant_index) => {} - - ProjectionElem::Field(..) => { - let base_ty = Place::ty_from(place.local, &proj_base, body, tcx).ty; - if let Some(def) = base_ty.ty_adt_def() { - // No union field accesses in `const fn` - if def.is_union() { - if !feature_allowed(tcx, def_id, sym::const_fn_union) { - return Err((span, "accessing union fields is unstable".into())); - } - } - } - } - ProjectionElem::ConstantIndex { .. } - | ProjectionElem::Subslice { .. } - | ProjectionElem::Deref - | ProjectionElem::Index(_) => {} - } - } - - Ok(()) -} - -/// Returns `true` if the given feature gate is allowed within the function with the given `DefId`. -fn feature_allowed(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bool { - // All features require that the corresponding gate be enabled, - // even if the function has `#[allow_internal_unstable(the_gate)]`. - if !tcx.features().enabled(feature_gate) { - return false; - } - - // If this crate is not using stability attributes, or this function is not claiming to be a - // stable `const fn`, that is all that is required. - if !tcx.features().staged_api || tcx.has_attr(def_id, sym::rustc_const_unstable) { - return true; - } - - // However, we cannot allow stable `const fn`s to use unstable features without an explicit - // opt-in via `allow_internal_unstable`. - attr::allow_internal_unstable(&tcx.get_attrs(def_id), &tcx.sess.diagnostic()) - .map_or(false, |mut features| features.any(|name| name == feature_gate)) -} - -fn check_terminator( - tcx: TyCtxt<'tcx>, - body: &'a Body<'tcx>, - def_id: DefId, - terminator: &Terminator<'tcx>, -) -> McfResult { - let span = terminator.source_info.span; - match &terminator.kind { - TerminatorKind::FalseEdges { .. } - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::Goto { .. } - | TerminatorKind::Return - | TerminatorKind::Resume => Ok(()), - - TerminatorKind::Drop { location, .. } => check_place(tcx, location, span, def_id, body), - TerminatorKind::DropAndReplace { location, value, .. } => { - check_place(tcx, location, span, def_id, body)?; - check_operand(tcx, value, span, def_id, body) - } - - TerminatorKind::SwitchInt { .. } if !feature_allowed(tcx, def_id, sym::const_if_match) => { - Err((span, "loops and conditional expressions are not stable in const fn".into())) - } - - TerminatorKind::SwitchInt { discr, switch_ty: _, values: _, targets: _ } => { - check_operand(tcx, discr, span, def_id, body) - } - - // FIXME(ecstaticmorse): We probably want to allow `Unreachable` unconditionally. - TerminatorKind::Unreachable if feature_allowed(tcx, def_id, sym::const_if_match) => Ok(()), - - TerminatorKind::Abort | TerminatorKind::Unreachable => { - Err((span, "const fn with unreachable code is not stable".into())) - } - TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { - Err((span, "const fn generators are unstable".into())) - } - - TerminatorKind::Call { func, args, from_hir_call: _, destination: _, cleanup: _ } => { - let fn_ty = func.ty(body, tcx); - if let ty::FnDef(def_id, _) = fn_ty.kind { - if !crate::const_eval::is_min_const_fn(tcx, def_id) { - return Err(( - span, - format!( - "can only call other `const fn` within a `const fn`, \ - but `{:?}` is not stable as `const fn`", - func, - ) - .into(), - )); - } - - check_operand(tcx, func, span, def_id, body)?; - - for arg in args { - check_operand(tcx, arg, span, def_id, body)?; - } - Ok(()) - } else { - Err((span, "can only call other const fns within const fn".into())) - } - } - - TerminatorKind::Assert { cond, expected: _, msg: _, target: _, cleanup: _ } => { - check_operand(tcx, cond, span, def_id, body) - } - } -} From 5e3a03827996786ebb92001efa6175ced7d981e7 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 7 Feb 2020 14:22:37 -0800 Subject: [PATCH 3/5] Check for `&&` and `||` in the HIR const-checker --- Cargo.lock | 1 + .../transform/check_consts/validation.rs | 2 +- src/librustc_passes/Cargo.toml | 1 + src/librustc_passes/check_const.rs | 53 +++++++++++++++---- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bbfda0fa2c846..003074bff1983 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3851,6 +3851,7 @@ dependencies = [ "rustc_feature", "rustc_hir", "rustc_index", + "rustc_mir", "rustc_session", "rustc_span", "rustc_target", diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 620d1ee8f3d11..c0211b117c419 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -580,7 +580,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { StatementKind::InlineAsm { .. } => self.check_op(ops::InlineAsm), - // Try to ensure that no `match` expressions have gotten throught the HIR const-checker. + // Try to ensure that no `match` expressions have gotten through the HIR const-checker. StatementKind::FakeRead(FakeReadCause::ForMatchGuard, _) | StatementKind::FakeRead(FakeReadCause::ForGuardBinding, _) | StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => { diff --git a/src/librustc_passes/Cargo.toml b/src/librustc_passes/Cargo.toml index 981ef7f8796d3..e5f276fc905aa 100644 --- a/src/librustc_passes/Cargo.toml +++ b/src/librustc_passes/Cargo.toml @@ -16,6 +16,7 @@ rustc_data_structures = { path = "../librustc_data_structures" } rustc_errors = { path = "../librustc_errors" } rustc_feature = { path = "../librustc_feature" } rustc_hir = { path = "../librustc_hir" } +rustc_mir = { path = "../librustc_mir" } rustc_index = { path = "../librustc_index" } rustc_session = { path = "../librustc_session" } rustc_target = { path = "../librustc_target" } diff --git a/src/librustc_passes/check_const.rs b/src/librustc_passes/check_const.rs index 6aacd4d4be8eb..5e367fc99851e 100644 --- a/src/librustc_passes/check_const.rs +++ b/src/librustc_passes/check_const.rs @@ -16,6 +16,7 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; +use rustc_mir::const_eval::is_min_const_fn; use rustc_span::{sym, Span, Symbol}; use syntax::ast::Mutability; @@ -27,6 +28,8 @@ enum NonConstExpr { Loop(hir::LoopSource), Match(hir::MatchSource), OrPattern, + LogicalOr, + LogicalAnd, } impl NonConstExpr { @@ -35,6 +38,8 @@ impl NonConstExpr { Self::Loop(src) => format!("`{}`", src.name()), Self::Match(src) => format!("`{}`", src.name()), Self::OrPattern => format!("or-pattern"), + Self::LogicalOr => format!("`||`"), + Self::LogicalAnd => format!("`&&`"), } } @@ -46,6 +51,8 @@ impl NonConstExpr { Self::Match(Normal) | Self::Match(IfDesugar { .. }) | Self::Match(IfLetDesugar { .. }) + | Self::LogicalOr + | Self::LogicalAnd | Self::OrPattern => &[sym::const_if_match], Self::Loop(Loop) => &[sym::const_loop], @@ -64,26 +71,34 @@ impl NonConstExpr { } } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq, Eq)] enum ConstKind { Static, StaticMut, ConstFn, + MinConstFn, Const, AnonConst, } impl ConstKind { - fn for_body(body: &hir::Body<'_>, hir_map: Hir<'_>) -> Option { - let is_const_fn = |id| hir_map.fn_sig_by_hir_id(id).unwrap().header.is_const(); - - let owner = hir_map.body_owner(body.id()); - let const_kind = match hir_map.body_owner_kind(owner) { + fn for_body(tcx: TyCtxt<'_>, body: &hir::Body<'_>) -> Option { + let owner = tcx.hir().body_owner(body.id()); + let const_kind = match tcx.hir().body_owner_kind(owner) { hir::BodyOwnerKind::Const => Self::Const, hir::BodyOwnerKind::Static(Mutability::Mut) => Self::StaticMut, hir::BodyOwnerKind::Static(Mutability::Not) => Self::Static, - hir::BodyOwnerKind::Fn if is_const_fn(owner) => Self::ConstFn, + hir::BodyOwnerKind::Fn if is_min_const_fn(tcx, tcx.hir().local_def_id(owner)) => { + Self::MinConstFn + } + + // Use `is_const_fn_raw` here since we need to check the bodies of unstable `const fn` + // as well as stable ones. + hir::BodyOwnerKind::Fn if tcx.is_const_fn_raw(tcx.hir().local_def_id(owner)) => { + Self::ConstFn + } + hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => return None, }; @@ -97,7 +112,7 @@ impl fmt::Display for ConstKind { Self::Static => "static", Self::StaticMut => "static mut", Self::Const | Self::AnonConst => "const", - Self::ConstFn => "const fn", + Self::MinConstFn | Self::ConstFn => "const fn", }; write!(f, "{}", s) @@ -211,7 +226,7 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> { } fn visit_body(&mut self, body: &'tcx hir::Body<'tcx>) { - let kind = ConstKind::for_body(body, self.tcx.hir()); + let kind = ConstKind::for_body(self.tcx, body); self.recurse_into(kind, |this| intravisit::walk_body(this, body)); } @@ -229,6 +244,26 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> { // Skip the following checks if we are not currently in a const context. _ if self.const_kind.is_none() => {} + // Short-circuiting operators were forbidden outright in the min_const_fn checks. In + // other contexts, they are converted to non-short-circuiting operators while lowering + // to MIR and marked as "control-flow destroyed". Bodies whose control-flow has been + // altered in this manner are rejected during MIR const-checking if they have any + // user-declared locals, since the user could observe the change like so: + // + // let mut altered_control_flow = false; + // true && { altered_control_flow = true; false } + hir::ExprKind::Binary(kind, _, _) if self.const_kind == Some(ConstKind::MinConstFn) => { + let expr = match kind.node { + hir::BinOpKind::And => Some(NonConstExpr::LogicalAnd), + hir::BinOpKind::Or => Some(NonConstExpr::LogicalOr), + _ => None, + }; + + if let Some(expr) = expr { + self.const_check_violated(expr, e.span); + } + } + hir::ExprKind::Loop(_, _, source) => { self.const_check_violated(NonConstExpr::Loop(*source), e.span); } From 2aa956e060b3612100d0287cab28c2a5aa3f787f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 7 Feb 2020 14:23:04 -0800 Subject: [PATCH 4/5] Move `&&` and `||` test into separate file --- .../ui/consts/min_const_fn/min_const_fn.rs | 4 ---- .../consts/min_const_fn/short_circuiting.rs | 6 ++++++ .../min_const_fn/short_circuiting.stderr | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/consts/min_const_fn/short_circuiting.rs create mode 100644 src/test/ui/consts/min_const_fn/short_circuiting.stderr diff --git a/src/test/ui/consts/min_const_fn/min_const_fn.rs b/src/test/ui/consts/min_const_fn/min_const_fn.rs index c3436d4840ac7..ec508774d1146 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn.rs +++ b/src/test/ui/consts/min_const_fn/min_const_fn.rs @@ -98,10 +98,6 @@ const fn foo30_2(x: *mut u32) -> usize { x as usize } const fn foo30_2_with_unsafe(x: *mut u32) -> usize { unsafe { x as usize } } //~^ ERROR casting pointers to ints is unstable const fn foo30_6() -> bool { let x = true; x } -const fn foo36(a: bool, b: bool) -> bool { a && b } -//~^ ERROR loops and conditional expressions are not stable in const fn -const fn foo37(a: bool, b: bool) -> bool { a || b } -//~^ ERROR loops and conditional expressions are not stable in const fn const fn inc(x: &mut i32) { *x += 1 } //~^ ERROR mutable references in const fn are unstable diff --git a/src/test/ui/consts/min_const_fn/short_circuiting.rs b/src/test/ui/consts/min_const_fn/short_circuiting.rs new file mode 100644 index 0000000000000..582a6a6e10ffa --- /dev/null +++ b/src/test/ui/consts/min_const_fn/short_circuiting.rs @@ -0,0 +1,6 @@ +const fn and(a: bool, b: bool) -> bool { a && b } +//~^ ERROR `&&` is not allowed in a `const fn` +const fn or(a: bool, b: bool) -> bool { a || b } +//~^ ERROR `||` is not allowed in a `const fn` + +fn main() {} diff --git a/src/test/ui/consts/min_const_fn/short_circuiting.stderr b/src/test/ui/consts/min_const_fn/short_circuiting.stderr new file mode 100644 index 0000000000000..8fac6def37ee8 --- /dev/null +++ b/src/test/ui/consts/min_const_fn/short_circuiting.stderr @@ -0,0 +1,21 @@ +error[E0658]: `&&` is not allowed in a `const fn` + --> $DIR/short_circuiting.rs:1:42 + | +LL | const fn and(a: bool, b: bool) -> bool { a && b } + | ^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/49146 + = help: add `#![feature(const_if_match)]` to the crate attributes to enable + +error[E0658]: `||` is not allowed in a `const fn` + --> $DIR/short_circuiting.rs:3:41 + | +LL | const fn or(a: bool, b: bool) -> bool { a || b } + | ^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/49146 + = help: add `#![feature(const_if_match)]` to the crate attributes to enable + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0658`. From cc70b25c1b155f476d1469ca3fdf78fe301b8a9d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 7 Feb 2020 17:03:36 -0800 Subject: [PATCH 5/5] Better name for `is_const_unstable` --- src/librustc_mir/transform/check_consts/mod.rs | 7 ------- src/librustc_mir/transform/check_consts/ops.rs | 15 +++++++++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs index 41c088153c80f..b7383663932a4 100644 --- a/src/librustc_mir/transform/check_consts/mod.rs +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -111,10 +111,3 @@ impl fmt::Display for ConstKind { pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { Some(def_id) == tcx.lang_items().panic_fn() || Some(def_id) == tcx.lang_items().begin_panic_fn() } - -/// Returns `true` if the constness of this item is not stabilized, either because it is declared -/// with `#![rustc_const_unstable]` or because the item itself is unstable. -fn is_const_unstable(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { - tcx.lookup_const_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) - || tcx.lookup_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) -} diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs index e0f78f548972d..c84a3e3e50643 100644 --- a/src/librustc_mir/transform/check_consts/ops.rs +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -3,14 +3,14 @@ use rustc::mir; use rustc::session::config::nightly_options; use rustc::session::parse::feature_err; -use rustc::ty::Ty; +use rustc::ty::{Ty, TyCtxt}; use rustc_errors::struct_span_err; use rustc_hir::def_id::DefId; use rustc_hir::Constness; use rustc_span::symbol::sym; use rustc_span::{Span, Symbol}; -use super::{is_const_unstable, ConstKind, Item}; +use super::{ConstKind, Item}; /// An operation that is not *always* allowed in a const context. pub trait NonConstOp: std::fmt::Debug { @@ -512,6 +512,13 @@ impl NonConstOp for UnionAccess { } } +/// Returns `true` if the constness of this item is not stabilized, either because it is declared +/// with `#[rustc_const_unstable]` or because the item itself is unstable. +fn is_declared_const_unstable(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { + tcx.lookup_const_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) + || tcx.lookup_stability(def_id).map_or(false, |stab| stab.level.is_unstable()) +} + /// Returns `true` if the feature with the given gate is allowed within this const context. fn feature_allowed(item: &Item<'_, '_>, feature_gate: Symbol) -> bool { let Item { tcx, def_id, .. } = *item; @@ -528,7 +535,7 @@ fn feature_allowed(item: &Item<'_, '_>, feature_gate: Symbol) -> bool { return true; } - if is_const_unstable(item.tcx, item.def_id) { + if is_declared_const_unstable(item.tcx, item.def_id) { return true; } @@ -544,7 +551,7 @@ fn min_const_fn_checks_enabled(item: &Item<'_, '_>) -> bool { if item.tcx.features().staged_api { // All functions except for unstable ones need to pass the min const fn checks. This // includes private functions that are not marked unstable. - !is_const_unstable(item.tcx, item.def_id) + !is_declared_const_unstable(item.tcx, item.def_id) } else { // Crates that are not using stability attributes can use `#![feature(const_fn)]` to opt out of // the min_const_fn checks.