Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various aspects around let bindings inside const functions #56160

Merged
merged 25 commits into from
Dec 18, 2018
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4d2bed9
Stabilize `const_let` inside const functions
oli-obk Nov 22, 2018
dba5ba0
Update a test's diagnostics
oli-obk Nov 22, 2018
df2123c
Update compile-fail test
oli-obk Nov 22, 2018
52b67b1
Remove a bunch of now-unnecessary `const_let` feature gates
oli-obk Nov 23, 2018
ef38afc
Add a test for various const let features
oli-obk Nov 23, 2018
59c6c49
Also test the `const_let` feature gate in statics
oli-obk Nov 23, 2018
7ec3c10
Add tests for mutable borrows
oli-obk Nov 23, 2018
d62bcad
Allow `let` bindings everywhere
oli-obk Nov 24, 2018
507ea97
Properly name the flag for `&&` -> `&` conversion
oli-obk Nov 26, 2018
75ce28a
Show auto-applicable correction warning for short circuiting in const…
oli-obk Nov 26, 2018
d8ece18
Improve the error around short circuiting and let bindings
oli-obk Nov 26, 2018
866664c
Add a test for single variant matches
oli-obk Nov 26, 2018
8937faa
Reenable `const_let` feature gate
oli-obk Nov 26, 2018
16d2a92
Improve the diagnostic message
oli-obk Nov 26, 2018
ac47bd7
Fix a compile-fail test
oli-obk Nov 27, 2018
25d1c07
Remove unused feature gate from `libcore`
oli-obk Nov 27, 2018
172b428
Re-add accidentally deleted test
oli-obk Nov 27, 2018
42e5317
Add trailing newline
oli-obk Nov 27, 2018
9d2f97b
Test float assign ops
oli-obk Nov 27, 2018
e6e08c6
Fix rebase fallout
oli-obk Nov 30, 2018
07345f0
Undo a change that got lost in the larger refactorings
oli-obk Dec 12, 2018
2cb5e3d
Manually inline trivial function
oli-obk Dec 12, 2018
6ca2ad5
Correct documentation about `FakeRead`
oli-obk Dec 12, 2018
b678238
Properly worded diagnostic message
oli-obk Dec 18, 2018
d815e2b
Explain that lack of short circuiting support in constants is temporary
oli-obk Dec 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@
#![feature(const_slice_len)]
#![feature(const_str_as_bytes)]
#![feature(const_str_len)]
#![feature(const_let)]
#![feature(const_int_rotate)]
#![feature(const_int_wrapping)]
#![feature(const_int_sign)]
Expand Down
15 changes: 15 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ pub struct Mir<'tcx> {
/// This is used for the "rust-call" ABI.
pub spread_arg: Option<Local>,

/// Mark this MIR of a const context other than const functions as having converted a `&&` or
/// `||` expression into `&` or `|` respectively. This is problematic because if we ever stop
/// this conversion from happening and use short circuiting, we will cause the following code
/// to change the value of `x`: `let mut x = 42; false && { x = 55; true };`
///
/// List of places where control flow was destroyed. Used for error reporting.
pub control_flow_destroyed: Vec<(Span, String)>,

/// A span representing this MIR, for error reporting
pub span: Span,

Expand All @@ -167,6 +175,7 @@ impl<'tcx> Mir<'tcx> {
arg_count: usize,
upvar_decls: Vec<UpvarDecl>,
span: Span,
control_flow_destroyed: Vec<(Span, String)>,
) -> Self {
// We need `arg_count` locals, and one for the return place
assert!(
Expand All @@ -191,6 +200,7 @@ impl<'tcx> Mir<'tcx> {
spread_arg: None,
span,
cache: cache::Cache::new(),
control_flow_destroyed,
}
}

Expand Down Expand Up @@ -421,6 +431,7 @@ impl_stable_hash_for!(struct Mir<'tcx> {
arg_count,
upvar_decls,
spread_arg,
control_flow_destroyed,
span,
cache
});
Expand Down Expand Up @@ -1748,6 +1759,9 @@ pub enum StatementKind<'tcx> {
/// (e.g. inspecting constants and discriminant values), and the
/// kind of pattern it comes from. This is in order to adapt potential
/// error messages to these specific patterns.
///
/// Note that this also is emitted for regular `let` bindings to ensure that locals that are
/// never accessed still get some sanity checks for e.g. `let x: ! = ..;`
FakeRead(FakeReadCause, Place<'tcx>),

/// Write the discriminant for a variant to the enum Place.
Expand Down Expand Up @@ -2972,6 +2986,7 @@ BraceStructTypeFoldableImpl! {
arg_count,
upvar_decls,
spread_arg,
control_flow_destroyed,
span,
cache,
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ CloneTypeFoldableAndLiftImpls! {
usize,
::ty::layout::VariantIdx,
u64,
String,
::middle::region::Scope,
::syntax::ast::FloatTy,
::syntax::ast::NodeId,
Expand Down
20 changes: 11 additions & 9 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,15 +854,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

Mir::new(self.cfg.basic_blocks,
self.source_scopes,
ClearCrossCrate::Set(self.source_scope_local_data),
IndexVec::new(),
yield_ty,
self.local_decls,
self.arg_count,
self.upvar_decls,
self.fn_span
Mir::new(
self.cfg.basic_blocks,
self.source_scopes,
ClearCrossCrate::Set(self.source_scope_local_data),
IndexVec::new(),
yield_ty,
self.local_decls,
self.arg_count,
self.upvar_decls,
self.fn_span,
self.hir.control_flow_destroyed(),
)
}

Expand Down
8 changes: 8 additions & 0 deletions src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,13 +372,21 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
// FIXME(eddyb) use logical ops in constants when
// they can handle that kind of control-flow.
(hir::BinOpKind::And, hir::Constness::Const) => {
cx.control_flow_destroyed.push((
op.span,
"`&&` operator".into(),
));
ExprKind::Binary {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah. I don't think I realized we do this sort of conversion.

op: BinOp::BitAnd,
lhs: lhs.to_ref(),
rhs: rhs.to_ref(),
}
}
(hir::BinOpKind::Or, hir::Constness::Const) => {
cx.control_flow_destroyed.push((
op.span,
"`||` operator".into(),
));
ExprKind::Binary {
op: BinOp::BitOr,
lhs: lhs.to_ref(),
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_mir/hair/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {

/// True if this constant/function needs overflow checks.
check_overflow: bool,

/// See field with the same name on `Mir`
control_flow_destroyed: Vec<(Span, String)>,
}

impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -96,9 +99,13 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
constness,
body_owner_kind,
check_overflow,
control_flow_destroyed: Vec::new(),
}
}

pub fn control_flow_destroyed(self) -> Vec<(Span, String)> {
self.control_flow_destroyed
}
}

impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
Expand Down
12 changes: 8 additions & 4 deletions src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
local_decls_for_sig(&sig, span),
sig.inputs().len(),
vec![],
span
span,
vec![],
);

if let Some(..) = ty {
Expand Down Expand Up @@ -387,7 +388,8 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {
self.local_decls,
self.sig.inputs().len(),
vec![],
self.span
self.span,
vec![],
)
}

Expand Down Expand Up @@ -835,7 +837,8 @@ fn build_call_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
local_decls,
sig.inputs().len(),
vec![],
span
span,
vec![],
);
if let Abi::RustCall = sig.abi {
mir.spread_arg = Some(Local::new(sig.inputs().len()));
Expand Down Expand Up @@ -912,6 +915,7 @@ pub fn build_adt_ctor<'a, 'gcx, 'tcx>(infcx: &infer::InferCtxt<'a, 'gcx, 'tcx>,
local_decls,
sig.inputs().len(),
vec![],
span
span,
vec![],
)
}
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,8 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
initial_locals,
0,
vec![],
mir.span
mir.span,
vec![],
),
tcx,
source: mir,
Expand Down
38 changes: 35 additions & 3 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,13 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
this.super_place(place, context, location);
match proj.elem {
ProjectionElem::Deref => {
this.add(Qualif::NOT_CONST);
if context.is_mutating_use() {
// `not_const` errors out in const contexts
this.not_const()
} else {
// just make sure this doesn't get promoted
this.add(Qualif::NOT_CONST);
}
let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx);
match this.mode {
Mode::Fn => {},
Expand Down Expand Up @@ -1159,7 +1165,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
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))) |
Expand All @@ -1170,7 +1175,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}
_ => {}
}

// Avoid a generic error for other uses of arguments.
if self.qualif.contains(Qualif::FN_ARGUMENT) {
let decl = &self.mir.local_decls[index];
Expand Down Expand Up @@ -1329,6 +1333,34 @@ impl MirPass for QualifyAndPromoteConstants {
// Do the actual promotion, now that we know what's viable.
promote_consts::promote_candidates(mir, tcx, temps, candidates);
} else {
if !mir.control_flow_destroyed.is_empty() {
let mut locals = mir.vars_iter();
if let Some(local) = locals.next() {
let span = mir.local_decls[local].source_info.span;
let mut error = tcx.sess.struct_span_err(
span,
&format!(
"new features like let bindings are not permitted in {} \
which also use short circuiting operators",
mode,
),
);
for (span, kind) in mir.control_flow_destroyed.iter() {
error.span_note(
*span,
&format!("use of {} here", kind),
);
}
for local in locals {
let span = mir.local_decls[local].source_info.span;
error.span_note(
span,
"more locals defined here",
);
}
error.emit();
}
}
let promoted_temps = if mode == Mode::Const {
// Already computed by `mir_const_qualif`.
const_promoted_temps.unwrap()
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ fn check_statement(
check_rvalue(tcx, mir, rval, span)
}

StatementKind::FakeRead(..) => Err((span, "match in const fn is unstable".into())),
StatementKind::FakeRead(_, place) => check_place(tcx, mir, place, span, PlaceMode::Read),

// just an assignment
StatementKind::SetDiscriminant { .. } => Ok(()),
Expand Down
4 changes: 1 addition & 3 deletions src/test/compile-fail/const-fn-error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(const_fn)]
#![feature(const_fn, const_let)]

const X : usize = 2;

const fn f(x: usize) -> usize {
let mut sum = 0;
//~^ let bindings in constant functions are unstable
//~| statements in constant functions are unstable
for i in 0..x {
//~^ ERROR E0015
//~| ERROR E0019
Expand Down
26 changes: 0 additions & 26 deletions src/test/run-pass/ctfe/const-fn-destructuring-arg.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ unsafe impl Sync for Foo {}
static FOO: Foo = Foo(UnsafeCell::new(42));

static BAR: () = unsafe {
*FOO.0.get() = 5; //~ ERROR could not evaluate static initializer
*FOO.0.get() = 5; //~ ERROR contains unimplemented expression type
};

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
error[E0080]: could not evaluate static initializer
error[E0019]: static contains unimplemented expression type
--> $DIR/assign-to-static-within-other-static-2.rs:27:5
|
LL | *FOO.0.get() = 5; //~ ERROR could not evaluate static initializer
| ^^^^^^^^^^^^^^^^ tried to modify a static's initial value from another static's initializer
LL | *FOO.0.get() = 5; //~ ERROR contains unimplemented expression type
| ^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
For more information about this error, try `rustc --explain E0019`.
4 changes: 1 addition & 3 deletions src/test/ui/consts/const-eval/mod-static-with-const-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ fn foo() {}

static BAR: () = unsafe {
*FOO.0.get() = 5;
// we do not error on the above access, because that is not detectable statically. Instead,
// const evaluation will error when trying to evaluate it. Due to the error below, we never even
// attempt to const evaluate `BAR`, so we don't see the error
//~^ contains unimplemented expression

foo();
//~^ ERROR calls in statics are limited to constant functions, tuple structs and tuple variants
Expand Down
13 changes: 10 additions & 3 deletions src/test/ui/consts/const-eval/mod-static-with-const-fn.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
error[E0019]: static contains unimplemented expression type
--> $DIR/mod-static-with-const-fn.rs:29:5
|
LL | *FOO.0.get() = 5;
| ^^^^^^^^^^^^^^^^

error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants
--> $DIR/mod-static-with-const-fn.rs:34:5
--> $DIR/mod-static-with-const-fn.rs:32:5
|
LL | foo();
| ^^^^^

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0015`.
Some errors occurred: E0015, E0019.
For more information about an error, try `rustc --explain E0015`.
14 changes: 6 additions & 8 deletions src/test/ui/consts/const-fn-not-safe-for-const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ const fn get_Y_addr() -> &'static u32 {
}

const fn get() -> u32 {
let x = 22;
//~^ ERROR let bindings in constant functions are unstable
//~| ERROR statements in constant functions are unstable
let y = 44;
//~^ ERROR let bindings in constant functions are unstable
//~| ERROR statements in constant functions are unstable
let x = 22; //~ ERROR let bindings in constant functions are unstable
//~^ ERROR statements in constant functions
let y = 44; //~ ERROR let bindings in constant functions are unstable
//~^ ERROR statements in constant functions
x + y
//~^ ERROR let bindings in constant functions are unstable
//~| ERROR let bindings in constant functions are unstable
//~^ ERROR let bindings in constant functions are unstable
//~| ERROR let bindings in constant functions are unstable
}

fn main() {}
Loading