Skip to content

Commit

Permalink
Rollup merge of rust-lang#76807 - ecstatic-morse:const-checking-stage…
Browse files Browse the repository at this point in the history
…d-api, r=oli-obk

Use const-checking to forbid use of unstable features in const-stable functions

First step towards rust-lang#76618.

Currently this code isn't ever hit because `qualify_min_const_fn` runs first and catches pretty much everything. One exception is `const_precise_live_drops`, which does not use the newly added code since it runs as part of a separate pass.

Also contains some unrelated refactoring, which is split into separate commits.

r? @oli-obk
  • Loading branch information
ecstatic-morse authored Sep 22, 2020
2 parents b3433c7 + abc7167 commit 60b9901
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 71 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_mir/src/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
//! has interior mutability or needs to be dropped, as well as the visitor that emits errors when
//! it finds operations that are invalid in a certain context.
use rustc_attr as attr;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::mir;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Symbol;

pub use self::qualifs::Qualif;

Expand Down Expand Up @@ -55,3 +57,9 @@ impl ConstCx<'mir, 'tcx> {
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()
}

pub fn allow_internal_unstable(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bool {
let attrs = tcx.get_attrs(def_id);
attr::allow_internal_unstable(&tcx.sess, attrs)
.map_or(false, |mut features| features.any(|name| name == feature_gate))
}
142 changes: 88 additions & 54 deletions compiler/rustc_mir/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Concrete error types for all operations which may be invalid in a certain const context.
use rustc_errors::struct_span_err;
use rustc_errors::{struct_span_err, Applicability};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_session::config::nightly_options;
Expand All @@ -14,35 +14,54 @@ use super::ConstCx;
pub fn non_const<O: NonConstOp>(ccx: &ConstCx<'_, '_>, op: O, span: Span) {
debug!("illegal_op: op={:?}", op);

if op.is_allowed_in_item(ccx) {
return;
}
let gate = match op.status_in_item(ccx) {
Status::Allowed => return,

Status::Unstable(gate) if ccx.tcx.features().enabled(gate) => {
let unstable_in_stable = ccx.const_kind() == hir::ConstContext::ConstFn
&& ccx.tcx.features().enabled(sym::staged_api)
&& !ccx.tcx.has_attr(ccx.def_id.to_def_id(), sym::rustc_const_unstable)
&& !super::allow_internal_unstable(ccx.tcx, ccx.def_id.to_def_id(), gate);

if unstable_in_stable {
ccx.tcx.sess
.struct_span_err(span, &format!("`#[feature({})]` cannot be depended on in a const-stable function", gate.as_str()))
.span_suggestion(
ccx.body.span,
"if it is not part of the public API, make this function unstably const",
concat!(r#"#[rustc_const_unstable(feature = "...", issue = "...")]"#, '\n').to_owned(),
Applicability::HasPlaceholders,
)
.help("otherwise `#[allow_internal_unstable]` can be used to bypass stability checks")
.emit();
}

return;
}

Status::Unstable(gate) => Some(gate),
Status::Forbidden => None,
};

if ccx.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
ccx.tcx.sess.miri_unleashed_feature(span, O::feature_gate());
ccx.tcx.sess.miri_unleashed_feature(span, gate);
return;
}

op.emit_error(ccx, span);
}

pub enum Status {
Allowed,
Unstable(Symbol),
Forbidden,
}

/// An operation that is not *always* allowed in a const context.
pub trait NonConstOp: std::fmt::Debug {
/// Returns the `Symbol` corresponding to the feature gate that would enable this operation,
/// or `None` if such a feature gate does not exist.
fn feature_gate() -> Option<Symbol> {
None
}

/// Returns `true` if this operation is allowed in the given item.
///
/// This check should assume that we are not in a non-const `fn`, where all operations are
/// legal.
///
/// By default, it returns `true` if and only if this operation has a corresponding feature
/// gate and that gate is enabled.
fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool {
Self::feature_gate().map_or(false, |gate| ccx.tcx.features().enabled(gate))
/// Returns an enum indicating whether this operation is allowed within the given item.
fn status_in_item(&self, _ccx: &ConstCx<'_, '_>) -> Status {
Status::Forbidden
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand All @@ -53,9 +72,13 @@ pub trait NonConstOp: std::fmt::Debug {
"{} contains unimplemented expression type",
ccx.const_kind()
);
if let Some(feat) = Self::feature_gate() {
err.help(&format!("add `#![feature({})]` to the crate attributes to enable", feat));

if let Status::Unstable(gate) = self.status_in_item(ccx) {
if !ccx.tcx.features().enabled(gate) && nightly_options::is_nightly_build() {
err.help(&format!("add `#![feature({})]` to the crate attributes to enable", gate));
}
}

if ccx.tcx.sess.teach(&err.get_code().unwrap()) {
err.note(
"A function call isn't allowed in the const's initialization expression \
Expand Down Expand Up @@ -147,7 +170,9 @@ pub struct InlineAsm;
impl NonConstOp for InlineAsm {}

#[derive(Debug)]
pub struct LiveDrop(pub Option<Span>);
pub struct LiveDrop {
pub dropped_at: Option<Span>,
}
impl NonConstOp for LiveDrop {
fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
let mut diagnostic = struct_span_err!(
Expand All @@ -157,7 +182,7 @@ impl NonConstOp for LiveDrop {
"destructors cannot be evaluated at compile-time"
);
diagnostic.span_label(span, format!("{}s cannot evaluate destructors", ccx.const_kind()));
if let Some(span) = self.0 {
if let Some(span) = self.dropped_at {
diagnostic.span_label(span, "value is dropped here");
}
diagnostic.emit();
Expand All @@ -182,14 +207,13 @@ impl NonConstOp for CellBorrow {
#[derive(Debug)]
pub struct MutBorrow;
impl NonConstOp for MutBorrow {
fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool {
// Forbid everywhere except in const fn
ccx.const_kind() == hir::ConstContext::ConstFn
&& ccx.tcx.features().enabled(Self::feature_gate().unwrap())
}

fn feature_gate() -> Option<Symbol> {
Some(sym::const_mut_refs)
fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status {
// Forbid everywhere except in const fn with a feature gate
if ccx.const_kind() == hir::ConstContext::ConstFn {
Status::Unstable(sym::const_mut_refs)
} else {
Status::Forbidden
}
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand All @@ -201,15 +225,16 @@ impl NonConstOp for MutBorrow {
&format!("mutable references are not allowed in {}s", ccx.const_kind()),
)
} else {
struct_span_err!(
let mut err = struct_span_err!(
ccx.tcx.sess,
span,
E0764,
"mutable references are not allowed in {}s",
ccx.const_kind(),
)
);
err.span_label(span, format!("`&mut` is only allowed in `const fn`"));
err
};
err.span_label(span, "`&mut` is only allowed in `const fn`".to_string());
if ccx.tcx.sess.teach(&err.get_code().unwrap()) {
err.note(
"References in statics and constants may only refer \
Expand All @@ -226,11 +251,17 @@ impl NonConstOp for MutBorrow {
}
}

// FIXME(ecstaticmorse): Unify this with `MutBorrow`. It has basically the same issues.
#[derive(Debug)]
pub struct MutAddressOf;
impl NonConstOp for MutAddressOf {
fn feature_gate() -> Option<Symbol> {
Some(sym::const_mut_refs)
fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status {
// Forbid everywhere except in const fn with a feature gate
if ccx.const_kind() == hir::ConstContext::ConstFn {
Status::Unstable(sym::const_mut_refs)
} else {
Status::Forbidden
}
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand All @@ -247,16 +278,16 @@ impl NonConstOp for MutAddressOf {
#[derive(Debug)]
pub struct MutDeref;
impl NonConstOp for MutDeref {
fn feature_gate() -> Option<Symbol> {
Some(sym::const_mut_refs)
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_mut_refs)
}
}

#[derive(Debug)]
pub struct Panic;
impl NonConstOp for Panic {
fn feature_gate() -> Option<Symbol> {
Some(sym::const_panic)
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_panic)
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand Down Expand Up @@ -289,8 +320,8 @@ impl NonConstOp for RawPtrComparison {
#[derive(Debug)]
pub struct RawPtrDeref;
impl NonConstOp for RawPtrDeref {
fn feature_gate() -> Option<Symbol> {
Some(sym::const_raw_ptr_deref)
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_raw_ptr_deref)
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand All @@ -307,8 +338,8 @@ impl NonConstOp for RawPtrDeref {
#[derive(Debug)]
pub struct RawPtrToIntCast;
impl NonConstOp for RawPtrToIntCast {
fn feature_gate() -> Option<Symbol> {
Some(sym::const_raw_ptr_to_usize_cast)
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_raw_ptr_to_usize_cast)
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand All @@ -326,8 +357,12 @@ impl NonConstOp for RawPtrToIntCast {
#[derive(Debug)]
pub struct StaticAccess;
impl NonConstOp for StaticAccess {
fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool {
matches!(ccx.const_kind(), hir::ConstContext::Static(_))
fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status {
if let hir::ConstContext::Static(_) = ccx.const_kind() {
Status::Allowed
} else {
Status::Forbidden
}
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand Down Expand Up @@ -371,14 +406,13 @@ impl NonConstOp for ThreadLocalAccess {
#[derive(Debug)]
pub struct UnionAccess;
impl NonConstOp for UnionAccess {
fn is_allowed_in_item(&self, ccx: &ConstCx<'_, '_>) -> bool {
fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status {
// Union accesses are stable in all contexts except `const fn`.
ccx.const_kind() != hir::ConstContext::ConstFn
|| ccx.tcx.features().enabled(Self::feature_gate().unwrap())
}

fn feature_gate() -> Option<Symbol> {
Some(sym::const_fn_union)
if ccx.const_kind() != hir::ConstContext::ConstFn {
Status::Allowed
} else {
Status::Unstable(sym::const_fn_union)
}
}

fn emit_error(&self, ccx: &ConstCx<'_, '_>, span: Span) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_hir::def_id::LocalDefId;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, BasicBlock, Location};
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
use rustc_span::{sym, Span};

use super::ops;
use super::qualifs::{NeedsDrop, Qualif};
Expand All @@ -11,7 +11,12 @@ use super::ConstCx;

/// Returns `true` if we should use the more precise live drop checker that runs after drop
/// elaboration.
pub fn checking_enabled(tcx: TyCtxt<'tcx>) -> bool {
pub fn checking_enabled(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> bool {
// Const-stable functions must always use the stable live drop checker.
if tcx.features().staged_api && !tcx.has_attr(def_id.to_def_id(), sym::rustc_const_unstable) {
return false;
}

tcx.features().const_precise_live_drops
}

Expand All @@ -25,7 +30,7 @@ pub fn check_live_drops(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &mir::Body<
return;
}

if !checking_enabled(tcx) {
if !checking_enabled(tcx, def_id) {
return;
}

Expand All @@ -52,7 +57,7 @@ impl std::ops::Deref for CheckLiveDrops<'mir, 'tcx> {

impl CheckLiveDrops<'mir, 'tcx> {
fn check_live_drop(&self, span: Span) {
ops::non_const(self.ccx, ops::LiveDrop(None), span);
ops::non_const(self.ccx, ops::LiveDrop { dropped_at: None }, span);
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir/src/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
| TerminatorKind::DropAndReplace { place: dropped_place, .. } => {
// If we are checking live drops after drop-elaboration, don't emit duplicate
// errors here.
if super::post_drop_elaboration::checking_enabled(self.tcx) {
if super::post_drop_elaboration::checking_enabled(self.tcx, self.def_id) {
return;
}

Expand All @@ -576,7 +576,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {

if needs_drop {
self.check_op_spanned(
ops::LiveDrop(Some(terminator.source_info.span)),
ops::LiveDrop { dropped_at: Some(terminator.source_info.span) },
err_span,
);
}
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_mir/src/transform/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use rustc_attr as attr;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::mir::*;
Expand Down Expand Up @@ -344,8 +343,7 @@ fn feature_allowed(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbol) -> bo

// 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.sess, &tcx.get_attrs(def_id))
.map_or(false, |mut features| features.any(|name| name == feature_gate))
super::check_consts::allow_internal_unstable(tcx, def_id, feature_gate)
}

/// Returns `true` if the given library feature gate is allowed within the function with the given `DefId`.
Expand All @@ -364,8 +362,7 @@ pub fn lib_feature_allowed(tcx: TyCtxt<'tcx>, def_id: DefId, feature_gate: Symbo

// 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.sess, &tcx.get_attrs(def_id))
.map_or(false, |mut features| features.any(|name| name == feature_gate))
super::check_consts::allow_internal_unstable(tcx, def_id, feature_gate)
}

fn check_terminator(
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/miri_unleashed/box.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ help: skipping check for `const_mut_refs` feature
|
LL | &mut *(box 0)
| ^^^^^^^^^^^^^
help: skipping check for `const_mut_refs` feature
help: skipping check that does not even have a feature gate
--> $DIR/box.rs:10:5
|
LL | &mut *(box 0)
Expand Down
Loading

0 comments on commit 60b9901

Please sign in to comment.