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

Separate unsized_locals into "sound" and "unsound" (i.e. incorrectly implemented) parts #71694

Closed
pnkfelix opened this issue Apr 29, 2020 · 0 comments · Fixed by #78152
Closed
Assignees
Labels
F-unsized_locals `#![feature(unsized_locals)]` P-high High priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Apr 29, 2020

Spawned off of #71416

Here is the example from that issue's description:

This is a variant of #68304 that was not fixed by #71170:

#![feature(unsized_locals)]

use std::any::Any;

#[repr(align(256))]
#[allow(dead_code)]
struct A {
    v: u8
}

impl A {
    fn f(&self) -> *const A {
        assert_eq!(self as *const A as usize % 256, 0);
        self
    }
}

fn mk() -> Box<dyn Any> {
    Box::new(A { v: 4 })
}

fn main() {
    let x = *mk();
    let dwncst = x.downcast_ref::<A>().unwrap();
    let addr = dwncst.f();
    assert_eq!(addr as usize % 256, 0);
}

What we want is to factor unsized_locals into two features (either giving both fresh names or just allocating one fresh name, it doesn't matter all that much), where one of the two named features would only enable support for cases that we have actually implemented (correctly).

  • Specifically, we do not currently handle alignment of dyn Value because we would need to query that alignment dynamically and adjust the stack address accordingly (and allowing for such adjustment by allocating size + (align 1) ... which actually I guess would have to be done via alloca if we cannot put an upper-bound on align ... anyway I digress...)

Anyway, this issue is just about creating the new feature name(s), and splitting the current unsized_locals support accordingly, so that crates (specifically the internals of libstd) can opt into supporting just the construct that we know we have actually implemented properly.

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-unsized_locals `#![feature(unsized_locals)]` requires-nightly This issue requires a nightly compiler in some way. P-high High priority labels Apr 29, 2020
@spastorino spastorino self-assigned this Apr 30, 2020
@bors bors closed this as completed in 346aeef Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-unsized_locals `#![feature(unsized_locals)]` P-high High priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
2 participants