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

ICE with unsized associated type #60431

Closed
petertodd opened this issue May 1, 2019 · 2 comments · Fixed by #62585
Closed

ICE with unsized associated type #60431

petertodd opened this issue May 1, 2019 · 2 comments · Fixed by #62585
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-codegen Area: Code generation A-typesystem Area: The type system C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petertodd
Copy link
Contributor

use std::mem;

pub trait Arena {
    type Dyn : ?Sized;
}

pub struct DynRef {
    _dummy: [()],
}

pub struct Ref<T, A: Arena> {
    _value: T,
    _dyn_arena: A::Dyn,
}

pub struct Obstack;

impl Arena for Obstack {
    type Dyn = DynRef;
}

fn main() {
    mem::size_of::<&Ref<u8, Obstack>>();
}

produces:

$ rustc ice-dyn.rs 
error: internal compiler error: src/librustc_codegen_llvm/context.rs:867: failed to get layout for `&Ref<u8, Obstack>`: the type `<Obstack as Arena>::Dyn` has an unknown layout

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:620:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
error: aborting due to previous error


note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.34.1 (fc50f328b 2019-04-24) running on x86_64-unknown-linux-gnu

Some context as to what that's actually for: the idea was to use the len metadata of the slice within the DynRef to smuggle a pointer to the arena in question. At first glance you'd think you could also do:

pub struct Ref<T, A: Arena> {
    marker: PhantomData<A>,
    value: T,
    arena: [()],
}

But the problem is that is mutable borrows: if Ref implements DerefMut there's no way to both access the value and the arena at the same time. You need them separate, and deliberately public:

pub struct Ref<T, A: Arena> {
    pub this: T,
    pub arena: A::Dyn,
}

allowing implementations like:

impl<A: Arena> Foo {
    fn frob(self: &mut Ref<Foo, A>) {
        self.arena.get(self.this.bar);
    }
}

...and as for why the object can't just own a reference to the arena it's in, tl;dr: memmap;

@Centril Centril added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-associated-items Area: Associated items (types, constants & functions) A-typesystem Area: The type system A-codegen Area: Code generation labels May 1, 2019
@pnkfelix
Copy link
Member

pnkfelix commented May 2, 2019

triage: P-high due to ICE. Removing nomination since there's no much to discuss beyond severity.

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels May 2, 2019
@pnkfelix pnkfelix self-assigned this Jun 27, 2019
@pnkfelix
Copy link
Member

Discussed with eddyb on zulip; they identified the cause and a fix pretty readily. Working on putting in the fix now.

@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Jul 11, 2019
Centril added a commit to Centril/rust that referenced this issue Jul 13, 2019
…il-normalize-when-possible, r=eddyb

Make struct_tail normalize when possible

As noted in commit message: this replaces the existing methods to extract the struct tail(s) with new entry points that make the handling of normalization explicit.

Most of the places that call `struct_tail` are during codegen, post type-checking, and therefore they can get away with using `tcx.normalize_erasing_regions` (this is the entry point `struct_tail_erasing_lifetimes`)

For other cases that may arise, one can use the core method, which is parameterized over the normalization `Ty -> Ty` closure (`struct_tail_with_normalize`).

Or one can use the trivial entry point that does not normalization (`struct_tail_without_normalization`)

----

I spent a little while trying to make a test that exposed the bug via `impl Trait` rather than a projection, but I failed to find something that tripped up the current nightly `rustc`.
 * I have *not* spent any time trying to make tests that trip up the other places where `struct_tail` was previously being called. While I do think the task of making such tests could be worthwhile, I am simply running out of time. (Its also possible that the layout code is always the first point called, and thus it may be pointless to try to come up with such tests.)

I also spent a little time discussing with @eddyb where this code should live. They suggested moving `struct_tail` and its sibling `struct_lockstep_tails` to the `LayoutCx`.  But in the interest of time, I have left that refactoring (which may be questionable at this point) to a follow-up task.

----

Fix rust-lang#60431
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-codegen Area: Code generation A-typesystem Area: The type system C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants