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

Array lengths don't support generic parameters. #43408

Open
roblabla opened this Issue Jul 22, 2017 · 11 comments

Comments

Projects
None yet
8 participants
@roblabla
Copy link
Contributor

roblabla commented Jul 22, 2017

It would seem that when using size_of in const fn context, it fails to properly compute the size of generic types.

The following function fails

unsafe fn zeroed<T: Sized>() -> T {
    // First create an array that has the same size as T, initialized at 0
    let x = [0u8; std::mem::size_of::<T>()];
    // Then, transmute it to type T, and return it
    std::mem::transmute(x)
}

The error is the following :

error[E0277]: the trait bound `T: std::marker::Sized` is not satisfied
 --> src/main.rs:3:19
  |
3 |     let x = [0u8; std::mem::size_of::<T>()];
  |                   ^^^^^^^^^^^^^^^^^^^^^^ `T` does not have a constant size known at compile-time
  |
  = help: the trait `std::marker::Sized` is not implemented for `T`
  = help: consider adding a `where T: std::marker::Sized` bound
  = note: required by `std::mem::size_of`

This is very confusing because it complains that T doesn't have std::marker::Sized, even though it does have that bound.

Meta

rustc_version : rustc 1.20.0-nightly (ae98ebf 2017-07-20)

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jul 26, 2017

cc @eddyb -- was this intentional?

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jul 27, 2017

This is a limitation of array length - it can't use any parameters in scope - which also came up during the stabilization of associated consts (cc @withoutboats do we have a common issue to use for this?) and the decision taken was to stabilize associated consts without it.

The exact error here comes from the type parameter being resolved, but the ty::Generics and ty::GenericPredicates (bounds, including the default Sized on type parameters) of the array length are actually both empty. They have to be (for now), otherwise we get cycle errors.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jul 27, 2017

This example triggers a const-evaluation error instead (try on playpen):

fn test<T: ?Sized>() {
    [0u8; std::mem::size_of::<&T>()];
}

cc @GuillaumeGomez @oli-obk Why is the const-eval error printing broken still 😢?

@scottmcm

This comment has been minimized.

Copy link
Member

scottmcm commented Jul 31, 2017

This makes me sad, as it's the one thing I wanted to do with #42859 😢

@ExpHP

This comment was marked as outdated.

Copy link
Contributor

ExpHP commented Apr 14, 2018

Weird thing: You can... um... kind of work around it. (but perhaps you shouldn't)

pub trait ConstSizeOf: Sized {
    const SIZE: usize = ::std::mem::size_of::<Self>();
}

impl<T> ConstSizeOf for T { }

You get a, uh... warning... that sounds extremely serious and that seems to suggest that this will have a very high probability of blowing up in your face.

   Compiling playground v0.0.1 (file:///playground)
warning: constant evaluation error: the type `Self` has an unknown layout
 --> src/main.rs:2:25
  |
2 |     const SIZE: usize = ::std::mem::size_of::<Self>();
  |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(const_err)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.52 secs
     Running `target/debug/playground`

But, um...

it, uh...seems to work.

fn main() {
    // using println instead of assert_eq! to make sure it isn't just optimized
    // away as undefined behavior
    println!("{:?}", <i8>::SIZE); // prints 1
    println!("{:?}", <i16>::SIZE); // prints 2
    println!("{:?}", <i32>::SIZE); // prints 4
    println!("{:?}", <i64>::SIZE); // prints 8
    println!("{:?}", <Vec<()>>::SIZE); // prints 24
    println!("{:?}", <Vec<i8>>::SIZE); // prints 24

    fn test_vec<T>() {
        println!("{:?}", Vec::<T>::SIZE);
    }
    fn test_tuple<T>() {
        println!("{:?}", <(T, T)>::SIZE);
    }
    test_vec::<()>(); // prints 24
    test_vec::<i8>(); // prints 24
    test_tuple::<()>(); // prints 0
    test_tuple::<i8>(); // prints 2
}
@ExpHP

This comment was marked as outdated.

Copy link
Contributor

ExpHP commented Apr 14, 2018

Oops, never mind.

// error: T: Sized is not satisfied
fn to_byte_array<T>() -> [u8; T::SIZE] {
    panic!()
}

I could have sworn I've done something before with associated consts in array types, though...

@juchiast

This comment has been minimized.

Copy link
Contributor

juchiast commented Apr 15, 2018

I think we shoud change the error message, this is very confusing now.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Apr 17, 2018

@ExpHP The problem is using type parameters (e.g. your T) in array lengths, everything else works.

@juchiast The error message is "emergent" from the same reason we can't "just" allow this to work right now, the the only other solution I can think of is checking if type-parameters are "really in scope" but that would probably break existing code that doesn't need to look at type parameters.

@eddyb eddyb changed the title const fn size_of doesn't work on generic types Array lengths don't support generic type parameters. Apr 17, 2018

@MageSlayer

This comment has been minimized.

Copy link

MageSlayer commented May 24, 2018

#![feature(const_fn)]
pub const fn sof<T:?Sized>() -> usize {
    10
}
fn to_byte_array<T>() -> [u8; sof::<T>()] {
     panic!()
}

Trying this way results in compiler crash in nightly.

error: internal compiler error: librustc/ty/subst.rs:456: Type parameter `T/#0` (T/0) out of range when substituting (root type=Some(fn() -> usize {sof::<T>})) substs=[]

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:499:9

Any workarounds known?

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented May 24, 2018

@MageSlayer Nope, it just can't be supported yet, see #43408 (comment) and previous.

bors added a commit that referenced this issue Sep 19, 2018

Auto merge of #54346 - eddyb:constant-horror, r=<try>
rustc: future-proof error reporting for polymorphic constants in types.

Currently, we have 3 categories of positions where a constant can be used (`const` and associated `const` can be considered "aliases" for an expression):
* runtime - if the function is polymorphic, we could even just warn and emit a panic
* `static` - always monomorphic, so we can error at definition site
* type-system - **must** *enforce* evaluation success where it was written

That last one is the tricky one, because we can't easily turn *the presence* a type with an erroring const into a runtime panic, and we'd have to do post-monomorphization errors (which we'd rather avoid).

<hr/>

The solution we came up with, as part of the plans for const generics, is to require successful evaluation wherever a constant shows up in a type (currently in array lengths, and values for const parameters in the future), *through* the WF system, which means that in certain situations (e.g. function signatures) we can assume evaluation *will* succeed, and require it of users (e.g. callers) instead (we've been doing this for lifetime bounds, for a long time now, and it's pretty ergonomic).

So once we fix #43408, this example *should* work, by propagating the responsability, to callers of `foo::<X>`, of proving `std::mem::size_of::<X>()` succeeds (and those callers can do the same).
```rust
pub fn foo<T>(_: [u8; std::mem::size_of::<T>()]) {}
```
But this one *shouldn't*, as there is nothing in the signature/bounds to indicate it:
```rust
pub fn bar<T>() {
    let _: [u8; std::mem::size_of::<T>()];
}
```

<hr/>

I've come across some bit of code in the compiler that ignores const-evaluation errors *even when* they come from a constant in a type, and I've added an ICE *only when* there are no other reported errors (e.g. it's fine to ignore evaluation errors if the constant doesn't even type-check).

r? @nikomatsakis cc @oli-obk @RalfJung @Centril

kennytm added a commit to kennytm/rust that referenced this issue Sep 20, 2018

Rollup merge of rust-lang#54346 - eddyb:constant-horror, r=nikomatsakis
rustc: future-proof error reporting for polymorphic constants in types.

Currently, we have 3 categories of positions where a constant can be used (`const` and associated `const` can be considered "aliases" for an expression):
* runtime - if the function is polymorphic, we could even just warn and emit a panic
* `static` - always monomorphic, so we can error at definition site
* type-system - **must** *enforce* evaluation success where it was written

That last one is the tricky one, because we can't easily turn *the presence* a type with an erroring const into a runtime panic, and we'd have to do post-monomorphization errors (which we'd rather avoid).

<hr/>

The solution we came up with, as part of the plans for const generics, is to require successful evaluation wherever a constant shows up in a type (currently in array lengths, and values for const parameters in the future), *through* the WF system, which means that in certain situations (e.g. function signatures) we can assume evaluation *will* succeed, and require it of users (e.g. callers) instead (we've been doing this for lifetime bounds, for a long time now, and it's pretty ergonomic).

So once we do sth about rust-lang#43408, this example *should* work, by propagating the responsability, to callers of `foo::<X>`, of proving `std::mem::size_of::<X>()` succeeds (and those callers can do the same).
```rust
pub fn foo<T>(_: [u8; std::mem::size_of::<T>()]) {}
```
But this one *shouldn't*, as there is nothing in the signature/bounds to indicate it:
```rust
pub fn bar<T>() {
    let _: [u8; std::mem::size_of::<T>()];
}
```

<hr/>

I've come across some bit of code in the compiler that ignores const-evaluation errors *even when* they come from a constant in a type, and I've added an ICE *only when* there are no other reported errors (e.g. it's fine to ignore evaluation errors if the constant doesn't even type-check).

r? @nikomatsakis cc @oli-obk @RalfJung @Centril

kennytm added a commit to kennytm/rust that referenced this issue Sep 20, 2018

Rollup merge of rust-lang#54346 - eddyb:constant-horror, r=nikomatsakis
rustc: future-proof error reporting for polymorphic constants in types.

Currently, we have 3 categories of positions where a constant can be used (`const` and associated `const` can be considered "aliases" for an expression):
* runtime - if the function is polymorphic, we could even just warn and emit a panic
* `static` - always monomorphic, so we can error at definition site
* type-system - **must** *enforce* evaluation success where it was written

That last one is the tricky one, because we can't easily turn *the presence* a type with an erroring const into a runtime panic, and we'd have to do post-monomorphization errors (which we'd rather avoid).

<hr/>

The solution we came up with, as part of the plans for const generics, is to require successful evaluation wherever a constant shows up in a type (currently in array lengths, and values for const parameters in the future), *through* the WF system, which means that in certain situations (e.g. function signatures) we can assume evaluation *will* succeed, and require it of users (e.g. callers) instead (we've been doing this for lifetime bounds, for a long time now, and it's pretty ergonomic).

So once we do sth about rust-lang#43408, this example *should* work, by propagating the responsability, to callers of `foo::<X>`, of proving `std::mem::size_of::<X>()` succeeds (and those callers can do the same).
```rust
pub fn foo<T>(_: [u8; std::mem::size_of::<T>()]) {}
```
But this one *shouldn't*, as there is nothing in the signature/bounds to indicate it:
```rust
pub fn bar<T>() {
    let _: [u8; std::mem::size_of::<T>()];
}
```

<hr/>

I've come across some bit of code in the compiler that ignores const-evaluation errors *even when* they come from a constant in a type, and I've added an ICE *only when* there are no other reported errors (e.g. it's fine to ignore evaluation errors if the constant doesn't even type-check).

r? @nikomatsakis cc @oli-obk @RalfJung @Centril

bors bot added a commit to tock/tock that referenced this issue Oct 17, 2018

Merge #1159
1159: New MPU Interface r=bradjc a=dverhaert

### Pull Request Overview

This pull request introduces an updated MPU interface that is no longer Cortex-M4 specific. It follows up on #1126, being part of a plan to make Tock architecture-agnostic (#985). 

An overview of the key differences:    

1. The interface now has two Tock-specific functions for allocating and updating the MPU region for the app-owned portion of its RAM.
2. The interface now only supports specifying user permissions for regions (instead of also specifying supervisor permissions). Memory locations not within any allocated region are required to be inaccessible in user mode, and accessible in supervisor mode. A related change is that we no longer allocate an MPU region for the grant memory in process.rs.
3. Instead of reallocating all the MPU regions every context switch, regions are now only allocated (computed by the MPU) when they are created or updated. Configuration of the MPU registers is now the only thing that happens during context switches.

The main reasoning behind the first decision is that the app break grows over the lifetime of the process. As such, we have found it infeasible for the client to request an MPU region that is suitable for this purpose through a generic region request. The reasoning behind the second decision is that allowing simultaneous specification of both user and supervisor permissions necessitates exposing a large matrix of options that no single MPU is able to support. When decoupled, we are left with a small, logical set of user permissions that we can reasonably expect all MPUs to be able to support. Furthermore, it is more consistent with our current practice of only enabling the MPU for processes.

The new interface allows removal of code from process.rs that assumed the presence of the Cortex-M MPU, and in particular its constraints and features. For instance, in the Cortex-M, the size of an MPU region must to be a power of two and aligned to the start address, and in the case of region overlap, the permissions of the overlapped segment are those of the region with the highest region number.

For further background and discussion, see the relevant [thread on tock-dev](https://groups.google.com/forum/#!topic/tock-dev/Fduxk_0xvUE). If something remains unclear, feel free to leave a comment below.

### Testing Strategy

This pull request was tested by running the `mpu_stack_growth` app in libtock-c, as well as a few others: one that walks over a process's RAM and flash regions, and several that try to make invalid accesses outside of these regions. 

### TODO or Help Wanted

- [ ] **Fix `_start` in userland**. @alevy is working on a [branch](https://github.com/dverhaert/libtock-c/tree/fix_start_order) in libtock-c that fixes `_start` so that processes never access memory past their app break. This pull request is blocking on that branch being merged. 
- [ ] **Loading processes**. Ideally we would load processes in decreasing order of RAM size on boards that use the Cortex-M MPU. Doing so would eliminate the possibility of external fragmentation between processes on such boards.
- [ ] **Size of the `mpu_regions` array**. This field in `struct Process` is currently hard-coded to be 6 in size because the Cortex-M supports 8 regions (and two of these are reserved for the flash and RAM regions). This is the last architecture-specific code remaining in process.rs. It currently seems infeasible to make this size chip-specific, seeing as [Rust doesn't seem to support this yet](rust-lang/rust#43408).

### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make formatall`.


Co-authored-by: Conor McAvity <cmcavity@protonmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Danilo Verhaert <daniloverhaert@gmail.com>
@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented Feb 17, 2019

Here is my workaround in Serde for [u8; mem::size_of::<T>()]. It supports rustc 1.20+. Note that the type is at least as big as T rather than exactly as big as T, so instead of transmute you would need ptr::write / ptr::read to interact with the bytes.

https://github.com/serde-rs/serde/blob/d07b62bba481af6603a736f027b5ebbb74a4a63a/serde/src/backport.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.