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

Implement Unsized Rvalues #51131

Merged
merged 10 commits into from
Aug 19, 2018
180 changes: 180 additions & 0 deletions src/doc/unstable-book/src/language-features/unsized-locals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
# `unsized_locals`

The tracking issue for this feature is: [#48055]

[#48055]: /~https://github.com/rust-lang/rust/issues/48055

------------------------

This implements [RFC1909]. When turned on, you can have unsized arguments and locals:

[RFC1909]: /~https://github.com/rust-lang/rfcs/blob/master/text/1909-coercions.md

```rust
#![feature(unsized_locals)]

use std::any::Any;

fn main() {
let x: Box<dyn Any> = Box::new(42);
let x: dyn Any = *x;
// ^ unsized local variable
// ^^ unsized temporary
foo(x);
}

fn foo(_: dyn Any) {}
// ^^^^^^ unsized argument
```

The RFC still forbids the following unsized expressions:

```rust,ignore
#![feature(unsized_locals)]

use std::any::Any;

struct MyStruct<T: ?Sized> {
content: T,
}

struct MyTupleStruct<T: ?Sized>(T);

fn answer() -> Box<dyn Any> {
Box::new(42)
}

fn main() {
// You CANNOT have unsized statics.
static X: dyn Any = *answer(); // ERROR
const Y: dyn Any = *answer(); // ERROR

// You CANNOT have struct initialized unsized.
MyStruct { content: *answer() }; // ERROR
MyTupleStruct(*answer()); // ERROR
(42, *answer()); // ERROR

// You CANNOT have unsized return types.
fn my_function() -> dyn Any { *answer() } // ERROR

// You CAN have unsized local variables...
let mut x: dyn Any = *answer(); // OK
// ...but you CANNOT reassign to them.
x = *answer(); // ERROR

// You CANNOT even initialize them separately.
let y: dyn Any; // OK
y = *answer(); // ERROR

// Not mentioned in the RFC, but by-move captured variables are also Sized.
let x: dyn Any = *answer();
(move || { // ERROR
let y = x;
})();

// You CAN create a closure with unsized arguments,
// but you CANNOT call it.
// This is an implementation detail and may be changed in the future.
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is because of rust-call and tuples? Annoying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, unfortunately. Args itself is Sized (at least for now) so we can't even pass an unsized rvalue in the last argument.

Copy link
Member

@eddyb eddyb Jul 13, 2018

Choose a reason for hiding this comment

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

You can remove the requirement from Args but we should maybe try to make it so it's possible for a tuple for have all elements !Sized but not be usable as a value or place type (I'm not sure how though).

let f = |x: dyn Any| {};
f(*answer()); // ERROR
}
```

However, the current implementation allows `MyTupleStruct(..)` to be unsized. This will be fixed in the future.

## By-value trait objects

With this feature, you can have by-value `self` arguments without `Self: Sized` bounds.

```rust
#![feature(unsized_locals)]

trait Foo {
fn foo(self) {}
}

impl<T: ?Sized> Foo for T {}

fn main() {
let slice: Box<[i32]> = Box::new([1, 2, 3]);
<[i32] as Foo>::foo(*slice);
}
```

And `Foo` will also be object-safe. However, this object-safety is not yet implemented.

```rust,ignore
#![feature(unsized_locals)]

trait Foo {
fn foo(self) {}
}

impl<T: ?Sized> Foo for T {}

fn main () {
let slice: Box<dyn Foo> = Box::new([1, 2, 3]);
// doesn't compile yet
<dyn Foo as Foo>::foo(*slice);
}
```

Unfortunately, this is not implemented yet.

One of the objectives of this feature is to allow `Box<dyn FnOnce>`, instead of `Box<dyn FnBox>` in the future. See [#28796] for details.

[#28796]: /~https://github.com/rust-lang/rust/issues/28796

## Variable length arrays

The RFC also describes an extension to the array literal syntax: `[e; dyn n]`. In the syntax, `n` isn't necessarily a constant expression. The array is dynamically allocated on the stack and has the type of `[T]`, instead of `[T; n]`.

```rust,ignore
#![feature(unsized_locals)]

fn mergesort<T: Ord>(a: &mut [T]) {
let mut tmp = [T; dyn a.len()];
// ...
}

fn main() {
let mut a = [3, 1, 5, 6];
mergesort(&mut a);
assert_eq!(a, [1, 3, 5, 6]);
}
```

VLAs are not implemented yet. The syntax isn't final, either. We may need an alternative syntax for Rust 2015 because, in Rust 2015, expressions like `[e; dyn(1)]` would be ambiguous. One possible alternative proposed in the RFC is `[e; n]`: if `n` captures one or more local variables, then it is considered as `[e; dyn n]`.

## Advisory on stack usage

It's advised not to casually use the `#![feature(unsized_locals)]` feature. Typical use-cases are:

- When you need a by-value trait objects.
- When you really need a fast allocation of small temporary arrays.

Another pitfall is repetitive allocation and temporaries. Currently the compiler simply extends the stack frame every time it encounters an unsized assignment. So for example, the code

```rust
#![feature(unsized_locals)]

fn main() {
let x: Box<[i32]> = Box::new([1, 2, 3, 4, 5]);
let _x = {{{{{{{{{{*x}}}}}}}}}};
}
```

and the code

```rust
#![feature(unsized_locals)]

fn main() {
for _ in 0..10 {
let x: Box<[i32]> = Box::new([1, 2, 3, 4, 5]);
let _x = *x;
}
}
```

will unnecessarily extend the stack frame.
9 changes: 9 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
ObligationCauseCode::VariableType(_) => {
err.note("all local variables must have a statically known size");
if !self.tcx.features().unsized_locals {
err.help("unsized locals are gated as an unstable feature");
}
}
ObligationCauseCode::SizedArgumentType => {
err.note("all function arguments must have a statically known size");
if !self.tcx.features().unsized_locals {
err.help("unsized locals are gated as an unstable feature");
}
}
ObligationCauseCode::SizedReturnType => {
err.note("the return type of a function must have a \
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ pub enum ObligationCauseCode<'tcx> {
StructInitializerSized,
/// Type of each variable must be Sized
VariableType(ast::NodeId),
/// Argument type must be Sized
SizedArgumentType,
/// Return type must be Sized
SizedReturnType,
/// Yield type must be Sized
Expand Down
1 change: 1 addition & 0 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::StructInitializerSized => Some(super::StructInitializerSized),
super::VariableType(id) => Some(super::VariableType(id)),
super::ReturnType(id) => Some(super::ReturnType(id)),
super::SizedArgumentType => Some(super::SizedArgumentType),
super::SizedReturnType => Some(super::SizedReturnType),
super::SizedYieldType => Some(super::SizedYieldType),
super::RepeatVec => Some(super::RepeatVec),
Expand Down
48 changes: 36 additions & 12 deletions src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,10 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
return;
}
let cx = bx.cx;
if self.is_indirect() {
OperandValue::Ref(val, self.layout.align).store(bx, dst)
if self.is_sized_indirect() {
OperandValue::Ref(val, None, self.layout.align).store(bx, dst)
} else if self.is_unsized_indirect() {
bug!("unsized ArgType must be handled through store_fn_arg");
} else if let PassMode::Cast(cast) = self.mode {
// FIXME(eddyb): Figure out when the simpler Store is safe, clang
// uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}.
Expand Down Expand Up @@ -246,7 +248,10 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
PassMode::Pair(..) => {
OperandValue::Pair(next(), next()).store(bx, dst);
}
PassMode::Direct(_) | PassMode::Indirect(_) | PassMode::Cast(_) => {
PassMode::Indirect(_, Some(_)) => {
OperandValue::Ref(next(), Some(next()), self.layout.align).store(bx, dst);
}
PassMode::Direct(_) | PassMode::Indirect(_, None) | PassMode::Cast(_) => {
self.store(bx, next(), dst);
}
}
Expand Down Expand Up @@ -302,6 +307,10 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
// Don't pass the vtable, it's not an argument of the virtual fn.
// Instead, pass just the (thin pointer) first field of `*dyn Trait`.
if arg_idx == Some(0) {
if layout.is_unsized() {
unimplemented!("by-value trait object is not \
yet implemented in #![feature(unsized_locals)]");
}
// FIXME(eddyb) `layout.field(cx, 0)` is not enough because e.g.
// `Box<dyn Trait>` has a few newtype wrappers around the raw
// pointer, so we'd have to "dig down" to find `*dyn Trait`.
Expand Down Expand Up @@ -538,7 +547,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
}

let size = arg.layout.size;
if size > layout::Pointer.size(cx) {
if arg.layout.is_unsized() || size > layout::Pointer.size(cx) {
arg.make_indirect();
} else {
// We want to pass small aggregates as immediates, but using
Expand All @@ -554,7 +563,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
for arg in &mut self.args {
fixup(arg);
}
if let PassMode::Indirect(ref mut attrs) = self.ret.mode {
if let PassMode::Indirect(ref mut attrs, _) = self.ret.mode {
attrs.set(ArgAttribute::StructRet);
}
return;
Expand All @@ -571,7 +580,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
if let PassMode::Pair(_, _) = arg.mode { 2 } else { 1 }
).sum();
let mut llargument_tys = Vec::with_capacity(
if let PassMode::Indirect(_) = self.ret.mode { 1 } else { 0 } + args_capacity
if let PassMode::Indirect(..) = self.ret.mode { 1 } else { 0 } + args_capacity
);

let llreturn_ty = match self.ret.mode {
Expand All @@ -580,7 +589,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
self.ret.layout.immediate_llvm_type(cx)
}
PassMode::Cast(cast) => cast.llvm_type(cx),
PassMode::Indirect(_) => {
PassMode::Indirect(..) => {
llargument_tys.push(self.ret.memory_ty(cx).ptr_to());
Type::void(cx)
}
Expand All @@ -600,8 +609,15 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Indirect(_, Some(_)) => {
let ptr_ty = cx.tcx.mk_mut_ptr(arg.layout.ty);
let ptr_layout = cx.layout_of(ptr_ty);
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true));
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Cast(cast) => cast.llvm_type(cx),
PassMode::Indirect(_) => arg.memory_ty(cx).ptr_to(),
PassMode::Indirect(_, None) => arg.memory_ty(cx).ptr_to(),
};
llargument_tys.push(llarg_ty);
}
Expand Down Expand Up @@ -640,7 +656,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
PassMode::Direct(ref attrs) => {
attrs.apply_llfn(llvm::AttributePlace::ReturnValue, llfn);
}
PassMode::Indirect(ref attrs) => apply(attrs),
PassMode::Indirect(ref attrs, _) => apply(attrs),
_ => {}
}
for arg in &self.args {
Expand All @@ -650,7 +666,11 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
match arg.mode {
PassMode::Ignore => {}
PassMode::Direct(ref attrs) |
PassMode::Indirect(ref attrs) => apply(attrs),
PassMode::Indirect(ref attrs, None) => apply(attrs),
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
apply(attrs);
apply(extra_attrs);
}
PassMode::Pair(ref a, ref b) => {
apply(a);
apply(b);
Expand All @@ -670,7 +690,7 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
PassMode::Direct(ref attrs) => {
attrs.apply_callsite(llvm::AttributePlace::ReturnValue, callsite);
}
PassMode::Indirect(ref attrs) => apply(attrs),
PassMode::Indirect(ref attrs, _) => apply(attrs),
_ => {}
}
if let layout::Abi::Scalar(ref scalar) = self.ret.layout.abi {
Expand All @@ -694,7 +714,11 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
match arg.mode {
PassMode::Ignore => {}
PassMode::Direct(ref attrs) |
PassMode::Indirect(ref attrs) => apply(attrs),
PassMode::Indirect(ref attrs, None) => apply(attrs),
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
apply(attrs);
apply(extra_attrs);
}
PassMode::Pair(ref a, ref b) => {
apply(a);
apply(b);
Expand Down
19 changes: 19 additions & 0 deletions src/librustc_codegen_llvm/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,25 @@ impl Builder<'a, 'll, 'tcx> {
}
}

pub fn array_alloca(&self,
ty: &'ll Type,
len: &'ll Value,
name: &str,
align: Align) -> &'ll Value {
self.count_insn("alloca");
unsafe {
let alloca = if name.is_empty() {
llvm::LLVMBuildArrayAlloca(self.llbuilder, ty, len, noname())
} else {
let name = SmallCStr::new(name);
llvm::LLVMBuildArrayAlloca(self.llbuilder, ty, len,
name.as_ptr())
};
llvm::LLVMSetAlignment(alloca, align.abi() as c_uint);
alloca
}
}

pub fn load(&self, ptr: &'ll Value, align: Align) -> &'ll Value {
self.count_insn("load");
unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ pub fn codegen_intrinsic_call(
// etc.
assert!(!bx.cx.type_needs_drop(arg.layout.ty));
let (ptr, align) = match arg.val {
OperandValue::Ref(ptr, align) => (ptr, align),
OperandValue::Ref(ptr, None, align) => (ptr, align),
_ => bug!()
};
let arg = PlaceRef::new_sized(ptr, arg.layout, align);
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_codegen_llvm/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,11 @@ extern "C" {

// Memory
pub fn LLVMBuildAlloca(B: &Builder<'a>, Ty: &'a Type, Name: *const c_char) -> &'a Value;
pub fn LLVMBuildArrayAlloca(B: &Builder<'a>,
Ty: &'a Type,
Val: &'a Value,
Name: *const c_char)
-> &'a Value;
pub fn LLVMBuildLoad(B: &Builder<'a>, PointerVal: &'a Value, Name: *const c_char) -> &'a Value;

pub fn LLVMBuildStore(B: &Builder<'a>, Val: &'a Value, Ptr: &'a Value) -> &'a Value;
Expand Down
Loading