Skip to content

Commit

Permalink
Rollup merge of rust-lang#57175 - oli-obk:const_let_stabilization, r=…
Browse files Browse the repository at this point in the history
…nikomatsakis

Stabilize `let` bindings and destructuring in constants and const fn

r? @Centril

This PR stabilizes the following features in constants and `const` functions:

* irrefutable destructuring patterns (e.g. `const fn foo((x, y): (u8, u8)) { ... }`)
* `let` bindings (e.g. `let x = 1;`)
* mutable `let` bindings (e.g. `let mut x = 1;`)
* assignment (e.g. `x = y`) and assignment operator (e.g. `x += y`) expressions, even where the assignment target is a projection (e.g. a struct field or index operation like `x[3] = 42`)
* expression statements (e.g. `3;`)

This PR does explicitly *not* stabilize:

* mutable references (i.e. `&mut T`)
* dereferencing mutable references
* refutable patterns (e.g. `Some(x)`)
* operations on `UnsafeCell` types (as that would need raw pointers and mutable references and such, not because it is explicitly forbidden. We can't explicitly forbid it as such values are OK as long as they aren't mutated.)
* We are not stabilizing `let` bindings in constants that use `&&` and `||` short circuiting operations. These are treated as `&` and `|` inside `const` and `static` items right now. If we stopped treating them as `&` and `|` after stabilizing `let` bindings, we'd break code like `let mut x = false; false && { x = true; false };`. So to use `let` bindings in constants you need to change `&&` and `||` to `&` and `|` respectively.
  • Loading branch information
Centril authored Jan 11, 2019
2 parents 729d3f0 + 16a4e47 commit 8f2c998
Show file tree
Hide file tree
Showing 90 changed files with 334 additions and 892 deletions.
224 changes: 42 additions & 182 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
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,7 @@ 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::Var if self.mode == Mode::Fn => {
self.add(Qualif::NOT_CONST);
}
LocalKind::Var |
Expand Down Expand Up @@ -569,6 +469,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}
}

ProjectionElem::ConstantIndex {..} |
ProjectionElem::Subslice {..} |
ProjectionElem::Field(..) |
ProjectionElem::Index(_) => {
let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx);
Expand Down Expand Up @@ -598,8 +500,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
this.qualif.restrict(ty, this.tcx, this.param_env);
}

ProjectionElem::ConstantIndex {..} |
ProjectionElem::Subslice {..} |
ProjectionElem::Downcast(..) => {
this.not_const()
}
Expand Down Expand Up @@ -1168,46 +1068,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
41 changes: 9 additions & 32 deletions src/librustc_mir/transform/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
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,29 +256,17 @@ 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::ConstantIndex { .. } | ProjectionElem::Subslice { .. }
| ProjectionElem::Deref | ProjectionElem::Field(..) | ProjectionElem::Index(_) => {
check_place(tcx, mir, &proj.base, span, mode)
}
// slice patterns are unstable
| ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {
return Err((span, "slice patterns in const fn are unstable".into()))
check_place(tcx, mir, &proj.base, span)
}
| ProjectionElem::Downcast(..) => {
Err((span, "`match` or `if let` in `const fn` is unstable".into()))
Expand All @@ -311,10 +288,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
Loading

0 comments on commit 8f2c998

Please sign in to comment.