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

Consider aggregate types containing unconstructable types to also be unconstructable #58374

Open
eira-fransham opened this issue Feb 11, 2019 · 8 comments
Labels
A-type-system Area: Type system needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@eira-fransham
Copy link

Currently Option<!> is 0-sized, but Option<(T, !)> isn't, despite the fact that the Some variant of the latter is unconstructable. If this were fixed then you could implement PhantomData in userland as:

type PhantomData<T> = Option<(T, !)>;

instead of it being special-cased in the compiler.

@estebank estebank added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Feb 11, 2019
@Centril Centril added needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. A-type-system Area: Type system labels Feb 11, 2019
@scottmcm
Copy link
Member

This has been discussed before. The complicated case is this one:

let x: (u32, !);
x.0 = 100;

That compiles today by actually writing the 100 into the first 4 bytes of the stack slot.

@Centril
Copy link
Contributor

Centril commented Feb 11, 2019

That compiles today by actually writing the 100 into the first 4 bytes of the stack slot.

Thankfully, it does not really (playground):

#![feature(never_type)]

fn main() {
    let mut x: (u32, !);
    x.0 = 100;
}

@scottmcm
Copy link
Member

@Centril That's absolutely what we tell LLVM to do, and what it does in debug mode (godbolt):

pub fn foo() {
    let x: (u32, !);
    x.0 = 100;
}
define void @_ZN7example3foo17hd23b272186309284E() unnamed_addr #0 !dbg !5 {
  %x = alloca { [0 x i32], i32, [0 x i8], { [0 x i8] }, [0 x i8] }, align 4
  %0 = bitcast { [0 x i32], i32, [0 x i8], { [0 x i8] }, [0 x i8] }* %x to i32*, !dbg !8
  store i32 100, i32* %0, align 4, !dbg !8
  ret void, !dbg !10
}
example::foo:
        push    rax
        mov     dword ptr [rsp], 100
        pop     rax
        ret

LLVM may optimize it sometimes (like it does let mut x: u32; x = 100;), but what I said is what we emit.

@Centril
Copy link
Contributor

Centril commented Feb 11, 2019

@scottmcm Sure, but this will eventually stop compiling so what we emit to LLVM is irrelevant because it won't reach LLVM and will fail during type checking instead.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Feb 11, 2019

We actually deliberately went from considering this a ZST to not considering it a ZST to fix bugs, see #49298

That issue also contains an example (I've seen more but I think most discussion is effectively lost in old chat protocols) that highlights that not uninhabited should not imply ZST, even if partial initialization is ruled out.

@vi
Copy link
Contributor

vi commented Mar 30, 2019

Maybe attempts to write to a sibling of an uninhabited field should be just no-ops?

With partial initialization layout adaptaions, ! loses its mathematical beauty and becomes just a fancier ZST.

! from my imagination turns Vec<!> and HashMap<i32,!> into ZSTs. #50622 seems like a step away from it.

If ! were added to the language at the same time as (), would it have pandered to partial initialisation?

@hanna-kruppe
Copy link
Contributor

cc rust-lang/unsafe-code-guidelines#79 and rust-lang/unsafe-code-guidelines#216 which also contain relevant discussion.

@vi
Copy link
Contributor

vi commented Feb 12, 2020

Maybe it can be opt-in for structs?

struct A {
    a: u32,
    b: !,
}

#[no_partial_initialisation]
struct B {
    a: u32,
    b: !,
}


fn main() {
    println!("{}", std::mem::size_of::<A>()); // 4
    loop { let _ = A { a: 23, b: break }; }
    
    println!("{}", std::mem::size_of::<A>()); // 0
    loop { let _ = B { a: 23, b: break }; }   // compile error
}

@fmease fmease added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants