Skip to content

Commit

Permalink
Stabilize let bindings and destructuring in constants and const fn
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Jan 9, 2019
1 parent 167ceff commit 4b4fc63
Show file tree
Hide file tree
Showing 86 changed files with 283 additions and 865 deletions.
221 changes: 41 additions & 180 deletions src/librustc_mir/transform/qualify_consts.rs
Expand Up @@ -21,7 +21,7 @@ use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUs
use rustc::middle::lang_items;
use rustc::session::config::nightly_options;
use syntax::ast::LitKind;
use syntax::feature_gate::{UnstableFeatures, feature_err, emit_feature_err, GateIssue};
use syntax::feature_gate::{UnstableFeatures, emit_feature_err, GateIssue};
use syntax_pos::{Span, DUMMY_SP};

use std::fmt;
Expand Down Expand Up @@ -104,7 +104,6 @@ struct Qualifier<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
param_env: ty::ParamEnv<'tcx>,
local_qualif: IndexVec<Local, Option<Qualif>>,
qualif: Qualif,
const_fn_arg_vars: BitSet<Local>,
temp_promotion_state: IndexVec<Local, TempState>,
promotion_candidates: Vec<Candidate>
}
Expand Down Expand Up @@ -139,7 +138,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
param_env,
local_qualif,
qualif: Qualif::empty(),
const_fn_arg_vars: BitSet::new_empty(mir.local_decls.len()),
temp_promotion_state: temps,
promotion_candidates: vec![]
}
Expand Down Expand Up @@ -168,26 +166,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
}
}

/// Error about extra statements in a constant.
fn statement_like(&mut self) {
self.add(Qualif::NOT_CONST);
if self.mode != Mode::Fn {
let mut err = feature_err(
&self.tcx.sess.parse_sess,
"const_let",
self.span,
GateIssue::Language,
&format!("statements in {}s are unstable", self.mode),
);
if self.tcx.sess.teach(&err.get_code().unwrap()) {
err.note("Blocks in constants may only contain items (such as constant, function \
definition, etc...) and a tail expression.");
err.help("To avoid it, you have to replace the non-item object.");
}
err.emit();
}
}

/// Add the given qualification to self.qualif.
fn add(&mut self, qualif: Qualif) {
self.qualif = self.qualif | qualif;
Expand Down Expand Up @@ -233,80 +211,46 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
return;
}

if self.tcx.features().const_let {
let mut dest = dest;
let index = loop {
match dest {
// with `const_let` active, we treat all locals equal
Place::Local(index) => break *index,
// projections are transparent for assignments
// we qualify the entire destination at once, even if just a field would have
// stricter qualification
Place::Projection(proj) => {
// Catch more errors in the destination. `visit_place` also checks various
// projection rules like union field access and raw pointer deref
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
dest = &proj.base;
},
Place::Promoted(..) => bug!("promoteds don't exist yet during promotion"),
Place::Static(..) => {
// Catch more errors in the destination. `visit_place` also checks that we
// do not try to access statics from constants or try to mutate statics
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
return;
}
let mut dest = dest;
let index = loop {
match dest {
// We treat all locals equal in constants
Place::Local(index) => break *index,
// projections are transparent for assignments
// we qualify the entire destination at once, even if just a field would have
// stricter qualification
Place::Projection(proj) => {
// Catch more errors in the destination. `visit_place` also checks various
// projection rules like union field access and raw pointer deref
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
dest = &proj.base;
},
Place::Promoted(..) => bug!("promoteds don't exist yet during promotion"),
Place::Static(..) => {
// Catch more errors in the destination. `visit_place` also checks that we
// do not try to access statics from constants or try to mutate statics
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
return;
}
};
debug!("store to var {:?}", index);
match &mut self.local_qualif[index] {
// this is overly restrictive, because even full assignments do not clear the qualif
// While we could special case full assignments, this would be inconsistent with
// aggregates where we overwrite all fields via assignments, which would not get
// that feature.
Some(ref mut qualif) => *qualif = *qualif | self.qualif,
// insert new qualification
qualif @ None => *qualif = Some(self.qualif),
}
return;
}

match *dest {
Place::Local(index) if self.mir.local_kind(index) == LocalKind::Temp ||
self.mir.local_kind(index) == LocalKind::ReturnPointer => {
debug!("store to {:?} (temp or return pointer)", index);
store(&mut self.local_qualif[index])
}

Place::Projection(box Projection {
base: Place::Local(index),
elem: ProjectionElem::Deref
}) if self.mir.local_kind(index) == LocalKind::Temp
&& self.mir.local_decls[index].ty.is_box()
&& self.local_qualif[index].map_or(false, |qualif| {
qualif.contains(Qualif::NOT_CONST)
}) => {
// Part of `box expr`, we should've errored
// already for the Box allocation Rvalue.
}

// This must be an explicit assignment.
_ => {
// Catch more errors in the destination.
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
self.statement_like();
}
};
debug!("store to var {:?}", index);
match &mut self.local_qualif[index] {
// this is overly restrictive, because even full assignments do not clear the qualif
// While we could special case full assignments, this would be inconsistent with
// aggregates where we overwrite all fields via assignments, which would not get
// that feature.
Some(ref mut qualif) => *qualif = *qualif | self.qualif,
// insert new qualification
qualif @ None => *qualif = Some(self.qualif),
}
}

Expand Down Expand Up @@ -347,45 +291,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
TerminatorKind::FalseUnwind { .. } => None,

TerminatorKind::Return => {
if !self.tcx.features().const_let {
// Check for unused values. This usually means
// there are extra statements in the AST.
for temp in mir.temps_iter() {
if self.local_qualif[temp].is_none() {
continue;
}

let state = self.temp_promotion_state[temp];
if let TempState::Defined { location, uses: 0 } = state {
let data = &mir[location.block];
let stmt_idx = location.statement_index;

// Get the span for the initialization.
let source_info = if stmt_idx < data.statements.len() {
data.statements[stmt_idx].source_info
} else {
data.terminator().source_info
};
self.span = source_info.span;

// Treat this as a statement in the AST.
self.statement_like();
}
}

// Make sure there are no extra unassigned variables.
self.qualif = Qualif::NOT_CONST;
for index in mir.vars_iter() {
if !self.const_fn_arg_vars.contains(index) {
debug!("unassigned variable {:?}", index);
self.assign(&Place::Local(index), Location {
block: bb,
statement_index: usize::MAX,
});
}
}
}

break;
}
};
Expand Down Expand Up @@ -454,12 +359,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
LocalKind::ReturnPointer => {
self.not_const();
}
LocalKind::Var if !self.tcx.features().const_let => {
if self.mode != Mode::Fn {
emit_feature_err(&self.tcx.sess.parse_sess, "const_let",
self.span, GateIssue::Language,
&format!("let bindings in {}s are unstable",self.mode));
}
LocalKind::Arg |
LocalKind::Var if self.mode == Mode::Fn => {
self.add(Qualif::NOT_CONST);
}
LocalKind::Var |
Expand Down Expand Up @@ -1168,46 +1069,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
debug!("visit_assign: dest={:?} rvalue={:?} location={:?}", dest, rvalue, location);
self.visit_rvalue(rvalue, location);

// Check the allowed const fn argument forms.
if let (Mode::ConstFn, &Place::Local(index)) = (self.mode, dest) {
if self.mir.local_kind(index) == LocalKind::Var &&
self.const_fn_arg_vars.insert(index) &&
!self.tcx.features().const_let {
// Direct use of an argument is permitted.
match *rvalue {
Rvalue::Use(Operand::Copy(Place::Local(local))) |
Rvalue::Use(Operand::Move(Place::Local(local))) => {
if self.mir.local_kind(local) == LocalKind::Arg {
return;
}
}
_ => {}
}
// Avoid a generic error for other uses of arguments.
if self.qualif.contains(Qualif::FN_ARGUMENT) {
let decl = &self.mir.local_decls[index];
let mut err = feature_err(
&self.tcx.sess.parse_sess,
"const_let",
decl.source_info.span,
GateIssue::Language,
"arguments of constant functions can only be immutable by-value bindings"
);
if self.tcx.sess.teach(&err.get_code().unwrap()) {
err.note("Constant functions are not allowed to mutate anything. Thus, \
binding to an argument with a mutable pattern is not allowed.");
err.note("Remove any mutable bindings from the argument list to fix this \
error. In case you need to mutate the argument, try lazily \
initializing a global variable instead of using a const fn, or \
refactoring the code to a functional style to avoid mutation if \
possible.");
}
err.emit();
return;
}
}
}

self.assign(dest, location);
}

Expand Down
36 changes: 8 additions & 28 deletions src/librustc_mir/transform/qualify_min_const_fn.rs
Expand Up @@ -65,12 +65,6 @@ pub fn is_min_const_fn(
}
}

for local in mir.vars_iter() {
return Err((
mir.local_decls[local].source_info.span,
"local variables in const fn are unstable".into(),
));
}
for local in &mir.local_decls {
check_ty(tcx, local.ty, local.source_info.span)?;
}
Expand Down Expand Up @@ -147,7 +141,7 @@ fn check_rvalue(
check_operand(tcx, mir, operand, span)
}
Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) => {
check_place(tcx, mir, place, span, PlaceMode::Read)
check_place(tcx, mir, place, span)
}
Rvalue::Cast(CastKind::Misc, operand, cast_ty) => {
use rustc::ty::cast::CastTy;
Expand Down Expand Up @@ -213,11 +207,6 @@ fn check_rvalue(
}
}

enum PlaceMode {
Assign,
Read,
}

fn check_statement(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &'a Mir<'tcx>,
Expand All @@ -226,11 +215,11 @@ fn check_statement(
let span = statement.source_info.span;
match &statement.kind {
StatementKind::Assign(place, rval) => {
check_place(tcx, mir, place, span, PlaceMode::Assign)?;
check_place(tcx, mir, place, span)?;
check_rvalue(tcx, mir, rval, span)
}

StatementKind::FakeRead(_, place) => check_place(tcx, mir, place, span, PlaceMode::Read),
StatementKind::FakeRead(_, place) => check_place(tcx, mir, place, span),

// just an assignment
StatementKind::SetDiscriminant { .. } => Ok(()),
Expand All @@ -256,7 +245,7 @@ fn check_operand(
) -> McfResult {
match operand {
Operand::Move(place) | Operand::Copy(place) => {
check_place(tcx, mir, place, span, PlaceMode::Read)
check_place(tcx, mir, place, span)
}
Operand::Constant(_) => Ok(()),
}
Expand All @@ -267,25 +256,16 @@ fn check_place(
mir: &'a Mir<'tcx>,
place: &Place<'tcx>,
span: Span,
mode: PlaceMode,
) -> McfResult {
match place {
Place::Local(l) => match mode {
PlaceMode::Assign => match mir.local_kind(*l) {
LocalKind::Temp | LocalKind::ReturnPointer => Ok(()),
LocalKind::Arg | LocalKind::Var => {
Err((span, "assignments in const fn are unstable".into()))
}
},
PlaceMode::Read => Ok(()),
},
Place::Local(_) => Ok(()),
// promoteds are always fine, they are essentially constants
Place::Promoted(_) => Ok(()),
Place::Static(_) => Err((span, "cannot access `static` items in const fn".into())),
Place::Projection(proj) => {
match proj.elem {
| ProjectionElem::Deref | ProjectionElem::Field(..) | ProjectionElem::Index(_) => {
check_place(tcx, mir, &proj.base, span, mode)
check_place(tcx, mir, &proj.base, span)
}
// slice patterns are unstable
| ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {
Expand All @@ -311,10 +291,10 @@ fn check_terminator(
| TerminatorKind::Resume => Ok(()),

TerminatorKind::Drop { location, .. } => {
check_place(tcx, mir, location, span, PlaceMode::Read)
check_place(tcx, mir, location, span)
}
TerminatorKind::DropAndReplace { location, value, .. } => {
check_place(tcx, mir, location, span, PlaceMode::Read)?;
check_place(tcx, mir, location, span)?;
check_operand(tcx, mir, value, span)
},

Expand Down
5 changes: 2 additions & 3 deletions src/libsyntax/feature_gate.rs
Expand Up @@ -193,9 +193,6 @@ declare_features! (
// Allows the definition of `const` functions with some advanced features.
(active, const_fn, "1.2.0", Some(24111), None),

// Allows let bindings and destructuring in `const` functions and constants.
(active, const_let, "1.22.1", Some(48821), None),

// Allows accessing fields of unions inside `const` functions.
(active, const_fn_union, "1.27.0", Some(51909), None),

Expand Down Expand Up @@ -688,6 +685,8 @@ declare_features! (
(accepted, min_const_unsafe_fn, "1.33.0", Some(55607), None),
// `#[cfg_attr(predicate, multiple, attributes, here)]`
(accepted, cfg_attr_multi, "1.33.0", Some(54881), None),
// Allows let bindings and destructuring in `const` functions and constants.
(accepted, const_let, "1.33.0", Some(48821), None),
);

// If you change this, please modify `src/doc/unstable-book` as well. You must
Expand Down

0 comments on commit 4b4fc63

Please sign in to comment.