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

Allow assignments in const contexts #56070

Merged
merged 15 commits into from
Nov 26, 2018
Merged
1 change: 1 addition & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ impl_stable_hash_for!(
CalledClosureAsFunction,
VtableForArgumentlessMethod,
ModifiedConstantMemory,
ModifiedStatic,
AssumptionNotHeld,
InlineAsm,
ReallocateNonBasePtr,
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ pub enum EvalErrorKind<'tcx, O> {
CalledClosureAsFunction,
VtableForArgumentlessMethod,
ModifiedConstantMemory,
ModifiedStatic,
AssumptionNotHeld,
InlineAsm,
TypeNotPrimitive(Ty<'tcx>),
Expand Down Expand Up @@ -380,6 +381,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
"tried to call a vtable function without arguments",
ModifiedConstantMemory =>
"tried to modify constant memory",
ModifiedStatic =>
"tried to modify a static's initial value from another static's initializer",
AssumptionNotHeld =>
"`assume` argument was false",
InlineAsm =>
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
Default +
Clone;

/// The memory kind to use for copied statics -- or None if those are not supported.
/// The memory kind to use for copied statics -- or None if statics should not be mutated
/// and thus any such attempt will cause a `ModifiedStatic` error is raised.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
/// Statics are copied under two circumstances: When they are mutated, and when
/// `static_with_default_tag` or `find_foreign_static` (see below) returns an owned allocation
/// that is added to the memory so that the work is not done twice.
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
if alloc.mutability == Mutability::Immutable {
return err!(ModifiedConstantMemory);
}
let kind = M::STATIC_KIND.expect(
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
"An allocation is being mutated but the machine does not expect that to happen"
);
Ok((MemoryKind::Machine(kind), alloc.into_owned()))
match M::STATIC_KIND {
Some(kind) => Ok((MemoryKind::Machine(kind), alloc.into_owned())),
None => err!(ModifiedStatic),
}
});
// Unpack the error type manually because type inference doesn't
// work otherwise (and we cannot help it because `impl Trait`)
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
| CalledClosureAsFunction
| VtableForArgumentlessMethod
| ModifiedConstantMemory
| ModifiedStatic
| AssumptionNotHeld
// FIXME: should probably be removed and turned into a bug! call
| TypeNotPrimitive(_)
Expand Down
39 changes: 33 additions & 6 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,30 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
return;
}

if self.tcx.features().const_let {
let mut dest = dest;
let index = loop {
match dest {
Place::Local(index) => break *index,
Place::Projection(proj) => dest = &proj.base,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Continuing with

the mindset when dealing with [const qualif] should be "how do I break this?"

Projections would include Deref, which isn't a problem because we can't have mutable references at all inside const contexts. Still something to think about.

Downcast also can't happen, because we'd need match for that.

Maybe we should just whitelist field projections for now.

Copy link
Member

@eddyb eddyb Nov 21, 2018

Choose a reason for hiding this comment

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

The problem is something like this:

const FOO: &(Cell<usize>, bool) = &{
    let mut foo = (Cell::new(0), false);
    foo.1 = true; // resets `qualif(foo)` to `qualif(true)`
    foo
};

I suspect that will actually compile with this PR, although I'd like a confirmation.

We could add this as a testcase, but you might be able to come up with something simpler/more general.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This indeed compiles successfully with this PR

Copy link
Member

Choose a reason for hiding this comment

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

My suggestion is field (or projections in general) assignment should combine qualifications (i.e. |=) instead of replacing them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now reuse the existing projection checks for reading values. These checks replace the qualification with NOT_CONST, so there's no real use in combining I think?

Place::Promoted(..) => bug!("promoteds don't exist yet during promotion"),
Place::Static(..) => {
// Catch more errors in the destination.
self.visit_place(
dest,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}
};
debug!("store to var {:?}", index);
self.local_qualif[index] = Some(self.qualif);
return;
}

match *dest {
Place::Local(index) if (self.mir.local_kind(index) == LocalKind::Var ||
self.mir.local_kind(index) == LocalKind::Arg) &&
self.tcx.sess.features_untracked().const_let => {
debug!("store to var {:?}", index);
self.local_qualif[index] = Some(self.qualif);
}
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);
Expand Down Expand Up @@ -478,6 +495,16 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {

// Only allow statics (not consts) to refer to other statics.
if self.mode == Mode::Static || self.mode == Mode::StaticMut {
if context.is_mutating_use() {
// this is not strictly necessary as miri will also bail out
// For interior mutability we can't really catch this statically as that
// goes through raw pointers and intermediate temporaries, so miri has
// to catch this anyway
self.tcx.sess.span_err(
self.span,
"cannot mutate statics in the initializer of another static",
);
}
return;
}
self.add(Qualif::NOT_CONST);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// New test for #53818: modifying static memory at compile-time is not allowed.
// The test should never compile successfully

#![feature(const_raw_ptr_deref)]
#![feature(const_let)]

use std::cell::UnsafeCell;

struct Foo(UnsafeCell<u32>);

unsafe impl Send for Foo {}
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
};

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0080]: could not evaluate static initializer
--> $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

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// New test for #53818: modifying static memory at compile-time is not allowed.
// The test should never compile successfully

#![feature(const_raw_ptr_deref)]
#![feature(const_let)]

use std::cell::UnsafeCell;

static mut FOO: u32 = 42;
static BOO: () = unsafe {
FOO = 5; //~ ERROR cannot mutate statics in the initializer of another static
};

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: cannot mutate statics in the initializer of another static
--> $DIR/assign-to-static-within-other-static.rs:21:5
|
LL | FOO = 5; //~ ERROR cannot mutate statics in the initializer of another static
| ^^^^^^^

error: aborting due to previous error

8 changes: 4 additions & 4 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 @@ -9,7 +9,7 @@
// except according to those terms.

// New test for #53818: modifying static memory at compile-time is not allowed.
// The test should never succeed.
// The test should never compile successfully

#![feature(const_raw_ptr_deref)]
#![feature(const_let)]
Expand All @@ -27,9 +27,9 @@ fn foo() {}

static BAR: () = unsafe {
*FOO.0.get() = 5;
//~^ ERROR statements in statics are unstable (see issue #48821)
// This error is caused by a separate bug that the feature gate error is reported
// even though the feature gate "const_let" is active.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
// 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

foo();
//~^ ERROR calls in statics are limited to constant functions, tuple structs and tuple variants
Expand Down
13 changes: 2 additions & 11 deletions src/test/ui/consts/const-eval/mod-static-with-const-fn.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
error[E0658]: statements in statics are unstable (see issue #48821)
--> $DIR/mod-static-with-const-fn.rs:29:5
|
LL | *FOO.0.get() = 5;
| ^^^^^^^^^^^^^^^^
|
= help: add #![feature(const_let)] to the crate attributes to enable

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

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

Some errors occurred: E0015, E0658.
For more information about an error, try `rustc --explain E0015`.
For more information about this error, try `rustc --explain E0015`.
12 changes: 12 additions & 0 deletions src/test/ui/consts/const_let_assign.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// compile-pass

#![feature(const_let)]

struct S(i32);

const A: () = {
let mut s = S(0);
s.0 = 1;
};

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/consts/const_let_assign2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// compile-pass

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

pub struct AA {
pub data: [u8; 10],
}

impl AA {
pub const fn new() -> Self {
let mut res: AA = AA { data: [0; 10] };
res.data[0] = 5;
res
}
}

static mut BB: AA = AA::new();

fn main() {
let ptr = unsafe { &mut BB };
for a in ptr.data.iter() {
println!("{}", a);
}
}
22 changes: 22 additions & 0 deletions src/test/ui/consts/const_let_assign3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![feature(const_let)]
#![feature(const_fn)]

struct S {
state: u32,
}

impl S {
const fn foo(&mut self, x: u32) {
self.state = x;
}
}

const FOO: S = {
let mut s = S { state: 42 };
s.foo(3); //~ ERROR references in constants may only refer to immutable values
s
};

fn main() {
assert_eq!(FOO.state, 3);
}
9 changes: 9 additions & 0 deletions src/test/ui/consts/const_let_assign3.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0017]: references in constants may only refer to immutable values
--> $DIR/const_let_assign3.rs:16:5
|
LL | s.foo(3); //~ ERROR references in constants may only refer to immutable values
| ^ constants require immutable values

error: aborting due to previous error

For more information about this error, try `rustc --explain E0017`.
14 changes: 14 additions & 0 deletions src/test/ui/consts/promote_const_let.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0597]: `y` does not live long enough
--> $DIR/promote_const_let.rs:6:9
|
LL | let x: &'static u32 = {
| ------------ type annotation requires that `y` is borrowed for `'static`
LL | let y = 42;
LL | &y //~ ERROR does not live long enough
| ^^ borrowed value does not live long enough
LL | };
| - `y` dropped here while still borrowed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
8 changes: 8 additions & 0 deletions src/test/ui/consts/promote_const_let.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![feature(const_let)]

fn main() {
let x: &'static u32 = {
let y = 42;
&y //~ ERROR does not live long enough
};
}
13 changes: 13 additions & 0 deletions src/test/ui/consts/promote_const_let.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0597]: `y` does not live long enough
--> $DIR/promote_const_let.rs:6:10
|
LL | &y //~ ERROR does not live long enough
| ^ borrowed value does not live long enough
LL | };
| - borrowed value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
10 changes: 8 additions & 2 deletions src/test/ui/error-codes/E0017.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ error[E0017]: references in constants may only refer to immutable values
LL | const CR: &'static mut i32 = &mut C; //~ ERROR E0017
| ^^^^^^ constants require immutable values

error: cannot mutate statics in the initializer of another static
--> $DIR/E0017.rs:15:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017
| ^^^^^^

error[E0017]: references in statics may only refer to immutable values
--> $DIR/E0017.rs:15:39
|
Expand All @@ -17,12 +23,12 @@ LL | static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017
| ^^^^^^ cannot borrow as mutable

error[E0017]: references in statics may only refer to immutable values
--> $DIR/E0017.rs:17:38
--> $DIR/E0017.rs:18:38
|
LL | static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017
| ^^^^^^ statics require immutable values

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

Some errors occurred: E0017, E0596.
For more information about an error, try `rustc --explain E0017`.
1 change: 1 addition & 0 deletions src/test/ui/error-codes/E0017.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ const C: i32 = 2;
const CR: &'static mut i32 = &mut C; //~ ERROR E0017
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0017
//~| ERROR cannot borrow
//~| ERROR cannot mutate statics
static CONST_REF: &'static mut i32 = &mut C; //~ ERROR E0017
fn main() {}
Loading