Skip to content

Commit

Permalink
Auto merge of #51131 - qnighy:unsized-locals, r=eddyb
Browse files Browse the repository at this point in the history
Implement Unsized Rvalues

This PR is the first step to implement RFC1909: unsized rvalues (#48055).

## Implemented

- `Sized` is removed for arguments and local bindings. (under `#![feature(unsized_locals)]`)
- Unsized locations are allowed in MIR
- Unsized places and operands are correctly translated at codegen

## Not implemented in this PR

- Additional `Sized` checks:
  - tuple struct constructor (accidentally compiles now)
  - closure arguments at closure generation (accidentally compiles now)
  - upvars (ICEs now)
- Generating vtable for `fn method(self)` (ICEs now)
- VLAs: `[e; n]` where `n` isn't const
- Reduce unnecessary allocations

## Current status

- [x] Fix `__rust_probestack` (rust-lang/compiler-builtins#244)
  - [x] Get the fix merged
- [x] `#![feature(unsized_locals)]`
  - [x] Give it a tracking issue number
- [x] Lift sized checks in typeck and MIR-borrowck
  - [ ] <del>Forbid `A(unsized-expr)`</del> will be another PR
- [x] Minimum working codegen
- [x] Add more examples and fill in unimplemented codegen paths
- [ ] <del>Loosen object-safety rules (will be another PR)</del>
- [ ] <del>Implement `Box<FnOnce>` (will be another PR)</del>
- [ ] <del>Reduce temporaries (will be another PR)</del>
  • Loading branch information
bors committed Aug 19, 2018
2 parents 8928de7 + c488d59 commit b355906
Show file tree
Hide file tree
Showing 40 changed files with 776 additions and 64 deletions.
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.
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

0 comments on commit b355906

Please sign in to comment.