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

desugaring-based placement-in (just in, no box); take-5 branch #27215

Merged
merged 27 commits into from
Jul 24, 2015
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1829fa5
Hack for "unsafety hygiene" -- `push_unsafe!` and `pop_unsafe!`.
pnkfelix Jun 5, 2015
866250c
prototype Placer protocol for unstable overloaded-box and placement-in.
pnkfelix Jan 27, 2015
d79bbbc
Revise placement-in expansion to use `push/pop_unsafe` and `move_val_…
pnkfelix Jun 5, 2015
b325e4f
Add feature-gates for desugaring-based `box` and placement-`in`.
pnkfelix Feb 12, 2015
75da81e
Change signature for `move_val_init` intrinsic to take `*mut T` for `…
pnkfelix Jun 5, 2015
cb5f0b4
Avoid feature-warnings on stage0.
pnkfelix Jun 5, 2015
07afe91
Allow unstable code to be injected by placement-`in` expansion.
pnkfelix Jun 5, 2015
9ca1c61
Instrumentation in effort to understand treatment of `allow_internal_…
pnkfelix Jun 5, 2015
bb95235
Factor feature gate tests for box syntax into two separate files.
pnkfelix Jul 21, 2015
4a5fb4b
Add necessary feature gate opt-in for the pushpop_unsafe test.
pnkfelix Jul 21, 2015
a9d7997
Update the `intrinsic-move-val.rs` test to reflect its newer signature.
pnkfelix Jul 21, 2015
8046533
Update `issue-14084.rs` test to reflect changes to error output.
pnkfelix Jul 21, 2015
e0b4479
Add new feature gate opt-in necessary for `new-box-syntax.rs`.
pnkfelix Jul 21, 2015
9f6f35b
Added support for parsing `in PLACE { BLOCK_CONTENT }`.
pnkfelix Jul 21, 2015
9d6cc05
uncomment feature-gate testing for `in PLACE BLOCK` now that its in t…
pnkfelix Jul 22, 2015
a81f88d
Add actual use of the `struct Structure` lying dormant in `new-box-sy…
pnkfelix Jul 22, 2015
7a70034
refine set of allowed warnings in `new-box-syntax.rs` test.
pnkfelix Jul 22, 2015
505cd8a
Add test of placement-in syntax, analogous to `new-box-syntax.rs`
pnkfelix Jul 22, 2015
abad7d6
placate `make tidy`.
pnkfelix Jul 22, 2015
1905a49
address review feedback: remove dupe feature opt-in.
pnkfelix Jul 22, 2015
73df224
Review feedback: add unstable marker to Placer API and put in bound t…
pnkfelix Jul 23, 2015
225298c
fix doc-tests by opting into `placement_in_syntax` feature where nece…
pnkfelix Jul 23, 2015
5682e2a
fix pretty printing tests by opting into the features that the expand…
pnkfelix Jul 23, 2015
565df57
Update suggestion from parenthesized-box-expr-message to reflect new …
pnkfelix Jul 23, 2015
44bb0dd
review feedback: Use checked-arith instead of saturated-arith for `pu…
pnkfelix Jul 23, 2015
2d68d09
review feedback: common-subexpression-elim across functions in pushpo…
pnkfelix Jul 23, 2015
d066a7b
update compile-fail/pushpop-unsafe-rejects.rs to reflect switch from …
pnkfelix Jul 23, 2015
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
107 changes: 102 additions & 5 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,16 @@

use core::prelude::*;

use heap;

use core::any::Any;
use core::cmp::Ordering;
use core::fmt;
use core::hash::{self, Hash};
use core::marker::Unsize;
use core::marker::{self, Unsize};
use core::mem;
use core::ops::{CoerceUnsized, Deref, DerefMut};
use core::ops::{Placer, Boxed, Place, InPlace, BoxPlace};
use core::ptr::Unique;
use core::raw::{TraitObject};

Expand All @@ -83,15 +86,110 @@ use core::raw::{TraitObject};
#[lang = "exchange_heap"]
#[unstable(feature = "box_heap",
reason = "may be renamed; uncertain about custom allocator design")]
pub const HEAP: () = ();
pub const HEAP: ExchangeHeapSingleton =
ExchangeHeapSingleton { _force_singleton: () };

/// This the singleton type used solely for `boxed::HEAP`.
#[derive(Copy, Clone)]
pub struct ExchangeHeapSingleton { _force_singleton: () }

/// A pointer type for heap allocation.
///
/// See the [module-level documentation](../../std/boxed/index.html) for more.
#[lang = "owned_box"]
#[stable(feature = "rust1", since = "1.0.0")]
#[fundamental]
pub struct Box<T>(Unique<T>);
pub struct Box<T: ?Sized>(Unique<T>);

/// `IntermediateBox` represents uninitialized backing storage for `Box`.
///
/// FIXME (pnkfelix): Ideally we would just reuse `Box<T>` instead of
/// introducing a separate `IntermediateBox<T>`; but then you hit
/// issues when you e.g. attempt to destructure an instance of `Box`,
/// since it is a lang item and so it gets special handling by the
/// compiler. Easier just to make this parallel type for now.
///
/// FIXME (pnkfelix): Currently the `box` protocol only supports
/// creating instances of sized types. This IntermediateBox is
/// designed to be forward-compatible with a future protocol that
/// supports creating instances of unsized types; that is why the type
/// parameter has the `?Sized` generalization marker, and is also why
/// this carries an explicit size. However, it probably does not need
/// to carry the explicit alignment; that is just a work-around for
/// the fact that the `align_of` intrinsic currently requires the
/// input type to be Sized (which I do not think is strictly
/// necessary).
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you expect align_of to return for a trait?

Copy link
Member

Choose a reason for hiding this comment

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

Presumably align_of would be bounded by an Aligned trait, a supertrait of Sized, implemented by all types except traits.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a non-issue because you can't allocate a box for a trait object (or at least, you first allocate a box for the concrete type). I think the use case we were primarily interested in was something like box [v; n] where n is NOT a compile-time constant. It's not clear how best to handle this (if we should at all), but it might be neat if it worked. (It would allocate enough space for an n-length array and initialize to 0.) Evaluation order is kind of weird, since you must evaluate n before v -- well, I guess that's not true, since we're going to be copying v into each spot, so we could still evaluate the v first. Anyway, I think that's the only case where it makes sense to allocate a box for an unsized type without having an actual value in hand.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I've been wanting this for ages, but it was blocked on having a placement API at all.
That is correct, you evaluate v first, then n, then allocate n * sizeof(v) bytes and only afterwards you can copy v into n different slots.
You can also do it for trait objects from Sized types: you allocate the size of the type, write the typed value in, then save the vtable pointer in the fat pointer without any unsizing step.

Direct allocation of unsized types is necessary to construct types that do not allow type-param-based unsizing, such as:

struct str([u8]);
struct Entity {
    id: u32,
    kind: Kind,
    name: &str,
    value: Any
}

Wouldn't &str(*b"foo") be a nice desugaring of "foo"? Admittedly, it couldn't be exposed everywhere without allowing invalid UTF-8. Unsafe fields/constructors, anyone?

#[unstable(feature = "placement_in", reason = "placement box design is still being worked out.")]
pub struct IntermediateBox<T: ?Sized>{
ptr: *mut u8,
size: usize,
align: usize,
marker: marker::PhantomData<*mut T>,
}

impl<T> Place<T> for IntermediateBox<T> {
fn pointer(&mut self) -> *mut T {
unsafe { ::core::mem::transmute(self.ptr) }
}
}

unsafe fn finalize<T>(b: IntermediateBox<T>) -> Box<T> {
let p = b.ptr as *mut T;
mem::forget(b);
mem::transmute(p)
}

fn make_place<T>() -> IntermediateBox<T> {
let size = mem::size_of::<T>();
let align = mem::align_of::<T>();

let p = if size == 0 {
heap::EMPTY as *mut u8
} else {
let p = unsafe {
heap::allocate(size, align)
};
if p.is_null() {
panic!("Box make_place allocation failure.");
}
p
};

IntermediateBox { ptr: p, size: size, align: align, marker: marker::PhantomData }
}

impl<T> BoxPlace<T> for IntermediateBox<T> {
fn make_place() -> IntermediateBox<T> { make_place() }
}

impl<T> InPlace<T> for IntermediateBox<T> {
type Owner = Box<T>;
unsafe fn finalize(self) -> Box<T> { finalize(self) }
}

impl<T> Boxed for Box<T> {
type Data = T;
type Place = IntermediateBox<T>;
unsafe fn finalize(b: IntermediateBox<T>) -> Box<T> { finalize(b) }
}

impl<T> Placer<T> for ExchangeHeapSingleton {
type Place = IntermediateBox<T>;

fn make_place(self) -> IntermediateBox<T> {
make_place()
}
}

impl<T: ?Sized> Drop for IntermediateBox<T> {
fn drop(&mut self) {
if self.size > 0 {
unsafe {
heap::deallocate(self.ptr, self.size, self.align)
}
}
}
}

impl<T> Box<T> {
/// Allocates memory on the heap and then moves `x` into it.
Expand Down Expand Up @@ -199,8 +297,7 @@ impl<T: Clone> Clone for Box<T> {
/// let y = x.clone();
/// ```
#[inline]
fn clone(&self) -> Box<T> { box {(**self).clone()} }

fn clone(&self) -> Box<T> { box (HEAP) {(**self).clone()} }
/// Copies `source`'s contents into `self` without creating a new allocation.
///
/// # Examples
Expand Down
2 changes: 2 additions & 0 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@
#![feature(no_std)]
#![feature(nonzero)]
#![feature(optin_builtin_traits)]
#![feature(placement_in_syntax)]
#![feature(raw)]
#![feature(staged_api)]
#![feature(placement_in_syntax)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate feature? (Should we have a lint for this?)

#![feature(unboxed_closures)]
#![feature(unique)]
#![feature(unsafe_no_drop_flag, filling_drop)]
Expand Down
8 changes: 8 additions & 0 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,14 @@ extern "rust-intrinsic" {
/// elements.
pub fn size_of<T>() -> usize;

#[cfg(not(stage0))]
/// Moves a value to an uninitialized memory location.
///
/// Drop glue is not run on the destination.
pub fn move_val_init<T>(dst: *mut T, src: T);

// SNAP ba0e1cd
#[cfg(stage0)]
/// Moves a value to an uninitialized memory location.
///
/// Drop glue is not run on the destination.
Expand Down
112 changes: 112 additions & 0 deletions src/libcore/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1266,3 +1266,115 @@ impl<T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<*const U> for *mut T {}

// *const T -> *const U
impl<T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<*const U> for *const T {}

/// Both `in (PLACE) EXPR` and `box EXPR` desugar into expressions
/// that allocate an intermediate "place" that holds uninitialized
/// state. The desugaring evaluates EXPR, and writes the result at
/// the address returned by the `pointer` method of this trait.
///
/// A `Place` can be thought of as a special representation for a
/// hypothetical `&uninit` reference (which Rust cannot currently
/// express directly). That is, it represents a pointer to
/// uninitialized storage.
///
/// The client is responsible for two steps: First, initializing the
/// payload (it can access its address via `pointer`). Second,
/// converting the agent to an instance of the owning pointer, via the
/// appropriate `finalize` method (see the `InPlace`.
///
/// If evaluating EXPR fails, then the destructor for the
/// implementation of Place to clean up any intermediate state
/// (e.g. deallocate box storage, pop a stack, etc).
pub trait Place<Data: ?Sized> {
/// Returns the address where the input value will be written.
/// Note that the data at this address is generally uninitialized,
/// and thus one should use `ptr::write` for initializing it.
fn pointer(&mut self) -> *mut Data;
}

/// Interface to implementations of `in (PLACE) EXPR`.
///
/// `in (PLACE) EXPR` effectively desugars into:
///
/// ```rust,ignore
/// let p = PLACE;
/// let mut place = Placer::make_place(p);
/// let raw_place = Place::pointer(&mut place);
/// let value = EXPR;
/// unsafe {
/// std::ptr::write(raw_place, value);
/// InPlace::finalize(place)
/// }
/// ```
///
/// The type of `in (PLACE) EXPR` is derived from the type of `PLACE`;
/// if the type of `PLACE` is `P`, then the final type of the whole
/// expression is `P::Place::Owner` (see the `InPlace` and `Boxed`
/// traits).
///
/// Values for types implementing this trait usually are transient
/// intermediate values (e.g. the return value of `Vec::emplace_back`)
/// or `Copy`, since the `make_place` method takes `self` by value.
pub trait Placer<Data: ?Sized> {
/// `Place` is the intermedate agent guarding the
/// uninitialized state for `Data`.
type Place: InPlace<Data>;

/// Creates a fresh place from `self`.
fn make_place(self) -> Self::Place;
}

/// Specialization of `Place` trait supporting `in (PLACE) EXPR`.
pub trait InPlace<Data: ?Sized>: Place<Data> {
/// `Owner` is the type of the end value of `in (PLACE) EXPR`
///
/// Note that when `in (PLACE) EXPR` is solely used for
/// side-effecting an existing data-structure,
/// e.g. `Vec::emplace_back`, then `Owner` need not carry any
/// information at all (e.g. it can be the unit type `()` in that
/// case).
type Owner;

/// Converts self into the final value, shifting
/// deallocation/cleanup responsibilities (if any remain), over to
/// the returned instance of `Owner` and forgetting self.
unsafe fn finalize(self) -> Self::Owner;
}

/// Core trait for the `box EXPR` form.
///
/// `box EXPR` effectively desugars into:
///
/// ```rust,ignore
/// let mut place = BoxPlace::make_place();
/// let raw_place = Place::pointer(&mut place);
/// let value = EXPR;
/// unsafe {
/// ::std::ptr::write(raw_place, value);
/// Boxed::finalize(place)
/// }
/// ```
///
/// The type of `box EXPR` is supplied from its surrounding
/// context; in the above expansion, the result type `T` is used
/// to determine which implementation of `Boxed` to use, and that
/// `<T as Boxed>` in turn dictates determines which
/// implementation of `BoxPlace` to use, namely:
/// `<<T as Boxed>::Place as BoxPlace>`.
pub trait Boxed {
/// The kind of data that is stored in this kind of box.
type Data; /* (`Data` unused b/c cannot yet express below bound.) */
/// The place that will negotiate the storage of the data.
type Place; /* should be bounded by BoxPlace<Self::Data> */

/// Converts filled place into final owning value, shifting
/// deallocation/cleanup responsibilities (if any remain), over to
/// returned instance of `Self` and forgetting `filled`.
unsafe fn finalize(filled: Self::Place) -> Self;
}

/// Specialization of `Place` trait supporting `box EXPR`.
pub trait BoxPlace<Data: ?Sized> : Place<Data> {
/// Creates a globally fresh place.
fn make_place() -> Self;
}
37 changes: 29 additions & 8 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

//! Enforces the Rust effect system. Currently there is just one effect,
//! `unsafe`.
use self::UnsafeContext::*;
use self::RootUnsafeContext::*;

use middle::def;
use middle::ty::{self, Ty};
Expand All @@ -21,8 +21,20 @@ use syntax::codemap::Span;
use syntax::visit;
use syntax::visit::Visitor;

#[derive(Copy, Clone)]
struct UnsafeContext {
push_unsafe_count: usize,
root: RootUnsafeContext,
}

impl UnsafeContext {
fn new(root: RootUnsafeContext) -> UnsafeContext {
UnsafeContext { root: root, push_unsafe_count: 0 }
}
}

#[derive(Copy, Clone, PartialEq)]
enum UnsafeContext {
enum RootUnsafeContext {
SafeContext,
UnsafeFn,
UnsafeBlock(ast::NodeId),
Expand All @@ -44,7 +56,8 @@ struct EffectCheckVisitor<'a, 'tcx: 'a> {

impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> {
fn require_unsafe(&mut self, span: Span, description: &str) {
match self.unsafe_context {
if self.unsafe_context.push_unsafe_count > 0 { return; }
match self.unsafe_context.root {
SafeContext => {
// Report an error.
span_err!(self.tcx.sess, span, E0133,
Expand Down Expand Up @@ -75,9 +88,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {

let old_unsafe_context = self.unsafe_context;
if is_unsafe_fn {
self.unsafe_context = UnsafeFn
self.unsafe_context = UnsafeContext::new(UnsafeFn)
} else if is_item_fn {
self.unsafe_context = SafeContext
self.unsafe_context = UnsafeContext::new(SafeContext)
}

visit::walk_fn(self, fn_kind, fn_decl, block, span);
Expand Down Expand Up @@ -105,10 +118,18 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
// external blocks (e.g. `unsafe { println("") }`,
// expands to `unsafe { ... unsafe { ... } }` where
// the inner one is compiler generated).
if self.unsafe_context == SafeContext || source == ast::CompilerGenerated {
self.unsafe_context = UnsafeBlock(block.id)
if self.unsafe_context.root == SafeContext || source == ast::CompilerGenerated {
self.unsafe_context.root = UnsafeBlock(block.id)
}
}
ast::PushUnsafeBlock(..) => {
self.unsafe_context.push_unsafe_count =
self.unsafe_context.push_unsafe_count.saturating_add(1);
}
ast::PopUnsafeBlock(..) => {
self.unsafe_context.push_unsafe_count =
self.unsafe_context.push_unsafe_count.saturating_sub(1);
}
}

visit::walk_block(self, block);
Expand Down Expand Up @@ -162,7 +183,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
pub fn check_crate(tcx: &ty::ctxt) {
let mut visitor = EffectCheckVisitor {
tcx: tcx,
unsafe_context: SafeContext,
unsafe_context: UnsafeContext::new(SafeContext),
};

visit::walk_crate(&mut visitor, tcx.map.krate());
Expand Down
5 changes: 5 additions & 0 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,11 @@ impl<'d,'t,'a,'tcx> ExprUseVisitor<'d,'t,'a,'tcx> {
None => {}
}
self.consume_expr(&**base);
if place.is_some() {
self.tcx().sess.span_bug(
expr.span,
"box with explicit place remains after expansion");
}
}

ast::ExprMac(..) => {
Expand Down
14 changes: 12 additions & 2 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,19 @@ pub fn check_pat(tcx: &ty::ctxt, pat: &ast::Pat,

fn maybe_do_stability_check(tcx: &ty::ctxt, id: ast::DefId, span: Span,
cb: &mut FnMut(ast::DefId, Span, &Option<&Stability>)) {
if !is_staged_api(tcx, id) { return }
if is_internal(tcx, span) { return }
if !is_staged_api(tcx, id) {
debug!("maybe_do_stability_check: \
skipping id={:?} since it is not staged_api", id);
return;
}
if is_internal(tcx, span) {
debug!("maybe_do_stability_check: \
skipping span={:?} since it is internal", span);
return;
}
let ref stability = lookup(tcx, id);
debug!("maybe_do_stability_check: \
inspecting id={:?} span={:?} of stability={:?}", id, span, stability);
cb(id, span, stability);
}

Expand Down
Loading