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

Tracking issue for RFC 1861: Extern types #43467

Open
aturon opened this Issue Jul 25, 2017 · 184 comments

Comments

Projects
None yet
@aturon
Copy link
Member

aturon commented Jul 25, 2017

This is a tracking issue for RFC 1861 "Extern types".

Steps:

Unresolved questions:

  • Should we allow generic lifetime and type parameters on extern types?
    If so, how do they effect the type in terms of variance?

  • In std's source, it is mentioned that LLVM expects i8* for C's void*.
    We'd need to continue to hack this for the two c_voids in std and libc.
    But perhaps this should be done across-the-board for all extern types?
    Somebody should check what Clang does. Also see #59095.

@aturon aturon changed the title Tracking issue for RFC 1861: Extern tyupes Tracking issue for RFC 1861: Extern types Jul 25, 2017

@aturon aturon referenced this issue Jul 25, 2017

Merged

extern types #1861

@jethrogb

This comment has been minimized.

Copy link
Contributor

jethrogb commented Jul 25, 2017

This is not explicitly mentioned in the RFC, but I'm assuming different instances of extern type are actually different types? Meaning this would be illegal:

extern {
    type A;
    type B;
}

fn convert_ref(r: &A) -> &B { r }
@canndrew

This comment has been minimized.

Copy link
Contributor

canndrew commented Jul 25, 2017

@jethrogb That's certainly the intention, yes.

@glaebhoerl

This comment has been minimized.

Copy link
Contributor

glaebhoerl commented Jul 25, 2017

Relatedly, is deciding whether we want to call it extern type or extern struct something that can still be done as part of the stabilization process, or is the extern type syntax effectively final as of having accepted the RFC?

EDIT: rust-lang/rfcs#2071 is also relevant here w.r.t. the connotations of type "aliases". In stable Rust a type declaration is "effect-free" and just a transparent alias for some existing type. Both extern type and type Foo = impl Bar would change this by making it implicitly generate a new module-scoped existential type or type constructor (nominal type) for it to refer to.

@Ericson2314

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Jul 25, 2017

Can we get a bullet for the panic vs DynSized debate?

@plietar

This comment has been minimized.

Copy link
Contributor

plietar commented Aug 10, 2017

I've started working on this, and I have a working simple initial version (no generics, no DynSized).

I've however noticed a slight usability issue. In FFI code, it's frequent for raw pointers to be initialized to null using std::ptr::null/null_mut. However, the function only accepts sized type arguments, since it would not be able to pick a metadata for the fat pointer.

Despite being unsized, extern types are used through thin pointers, so it should be possible to use std::ptr::null.

It is still possible to cast an integer to an extern type pointer, but this is not as nice as just using the function designed for this. Also this can never be done in a generic context.

extern {
    type foo;
}
fn null_foo() -> *const foo {
    0usize as *const foo
}

Really we'd want is a new trait to distinguish types which use thin pointers. It would be implemented automatically for all sized types and extern types. Then the cast above would succeed whenever the type is bounded by this trait. Eg, the function std::ptr::null becomes :

fn null<T: ?Sized + Thin>() -> *const T {
    0usize as *const T
}

However there's a risk of more and more such traits creeping up, such as DynSized, making it confusing for users. There's also some overlap with the various custom RFCs proposals which allow arbitrary metadata. For instance, instead of Thin, Referent<Meta=()> could be used

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Aug 10, 2017

I think we can add extern types now and live with str::ptr::null not supporting them for a while until we figure out what to do about Thin/DynSized/Referent<Meta=…> etc.

@plietar

This comment has been minimized.

Copy link
Contributor

plietar commented Aug 10, 2017

@SimonSapin yeah, it's definitely a minor concern for now.

I do think this problem of not having a trait bound to express "this type may be unsized be must have a thin pointer" might crop up in other places though.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Aug 11, 2017

Oh yeah, I agree we should solve that eventually too. I’m only saying we might not need to solve all of it before we ship any of it.

@plietar

This comment has been minimized.

Copy link
Contributor

plietar commented Sep 3, 2017

I've pushed an initial implementation in #44295

bors added a commit that referenced this issue Sep 4, 2017

Auto merge of #44295 - plietar:extern-types, r=arielb1
Implement RFC 1861: Extern types

A few notes :

- Type parameters are not supported. This was an unresolved question from the RFC. It is not clear how useful this feature is, and how variance should be treated. This can be added in a future PR.

- `size_of_val` / `align_of_val` can be called with extern types, and respectively return 0 and 1. This differs from the RFC, which specified that they should panic, but after discussion with @eddyb on IRC this seems like a better solution.
If/when a `DynSized` trait is added, this will be disallowed statically.

- Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are `!Sync`, `!Send` and `!Freeze`. This seems like the correct behaviour to me.
Manual `unsafe impl Sync for Foo` is still possible.

- This PR allows extern type to be used as the tail of a struct, as described by the RFC :
```rust
extern {
    type OpaqueTail;
}

#[repr(C)]
struct FfiStruct {
    data: u8,
    more_data: u32,
    tail: OpaqueTail,
}
```

However this is undesirable, as the alignment of `tail` is unknown (the current PR assumes an alignment of 1). Unfortunately we can't prevent it in the general case as the tail could be a type parameter :
```rust
#[repr(C)]
struct FfiStruct<T: ?Sized> {
    data: u8,
    more_data: u32,
    tail: T,
}
```

Adding a `DynSized` trait would solve this as well, by requiring tail fields to be bound by it.

- Despite being unsized, pointers to extern types are thin and can be casted from/to integers. However it is not possible to write a `null<T>() -> *const T` function which works with extern types, as I've explained here : #43467 (comment)

- Trait objects cannot be built from extern types. I intend to support it eventually, although how this interacts with `DynSized`/`size_of_val` is still unclear.

- The definition of `c_void` is unmodified

bors added a commit that referenced this issue Sep 10, 2017

Auto merge of #44295 - plietar:extern-types, r=arielb1
Implement RFC 1861: Extern types

A few notes :

- Type parameters are not supported. This was an unresolved question from the RFC. It is not clear how useful this feature is, and how variance should be treated. This can be added in a future PR.

- `size_of_val` / `align_of_val` can be called with extern types, and respectively return 0 and 1. This differs from the RFC, which specified that they should panic, but after discussion with @eddyb on IRC this seems like a better solution.
If/when a `DynSized` trait is added, this will be disallowed statically.

- Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are `!Sync`, `!Send` and `!Freeze`. This seems like the correct behaviour to me.
Manual `unsafe impl Sync for Foo` is still possible.

- This PR allows extern type to be used as the tail of a struct, as described by the RFC :
```rust
extern {
    type OpaqueTail;
}

#[repr(C)]
struct FfiStruct {
    data: u8,
    more_data: u32,
    tail: OpaqueTail,
}
```

However this is undesirable, as the alignment of `tail` is unknown (the current PR assumes an alignment of 1). Unfortunately we can't prevent it in the general case as the tail could be a type parameter :
```rust
#[repr(C)]
struct FfiStruct<T: ?Sized> {
    data: u8,
    more_data: u32,
    tail: T,
}
```

Adding a `DynSized` trait would solve this as well, by requiring tail fields to be bound by it.

- Despite being unsized, pointers to extern types are thin and can be casted from/to integers. However it is not possible to write a `null<T>() -> *const T` function which works with extern types, as I've explained here : #43467 (comment)

- Trait objects cannot be built from extern types. I intend to support it eventually, although how this interacts with `DynSized`/`size_of_val` is still unclear.

- The definition of `c_void` is unmodified

bors added a commit that referenced this issue Sep 13, 2017

Auto merge of #44295 - plietar:extern-types, r=arielb1
Implement RFC 1861: Extern types

A few notes :

- Type parameters are not supported. This was an unresolved question from the RFC. It is not clear how useful this feature is, and how variance should be treated. This can be added in a future PR.

- `size_of_val` / `align_of_val` can be called with extern types, and respectively return 0 and 1. This differs from the RFC, which specified that they should panic, but after discussion with @eddyb on IRC this seems like a better solution.
If/when a `DynSized` trait is added, this will be disallowed statically.

- Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are `!Sync`, `!Send` and `!Freeze`. This seems like the correct behaviour to me.
Manual `unsafe impl Sync for Foo` is still possible.

- This PR allows extern type to be used as the tail of a struct, as described by the RFC :
```rust
extern {
    type OpaqueTail;
}

#[repr(C)]
struct FfiStruct {
    data: u8,
    more_data: u32,
    tail: OpaqueTail,
}
```

However this is undesirable, as the alignment of `tail` is unknown (the current PR assumes an alignment of 1). Unfortunately we can't prevent it in the general case as the tail could be a type parameter :
```rust
#[repr(C)]
struct FfiStruct<T: ?Sized> {
    data: u8,
    more_data: u32,
    tail: T,
}
```

Adding a `DynSized` trait would solve this as well, by requiring tail fields to be bound by it.

- Despite being unsized, pointers to extern types are thin and can be casted from/to integers. However it is not possible to write a `null<T>() -> *const T` function which works with extern types, as I've explained here : #43467 (comment)

- Trait objects cannot be built from extern types. I intend to support it eventually, although how this interacts with `DynSized`/`size_of_val` is still unclear.

- The definition of `c_void` is unmodified

bors added a commit that referenced this issue Oct 1, 2017

Auto merge of #44295 - plietar:extern-types, r=arielb1
Implement RFC 1861: Extern types

A few notes :

- Type parameters are not supported. This was an unresolved question from the RFC. It is not clear how useful this feature is, and how variance should be treated. This can be added in a future PR.

- `size_of_val` / `align_of_val` can be called with extern types, and respectively return 0 and 1. This differs from the RFC, which specified that they should panic, but after discussion with @eddyb on IRC this seems like a better solution.
If/when a `DynSized` trait is added, this will be disallowed statically.

- Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are `!Sync`, `!Send` and `!Freeze`. This seems like the correct behaviour to me.
Manual `unsafe impl Sync for Foo` is still possible.

- This PR allows extern type to be used as the tail of a struct, as described by the RFC :
```rust
extern {
    type OpaqueTail;
}

#[repr(C)]
struct FfiStruct {
    data: u8,
    more_data: u32,
    tail: OpaqueTail,
}
```

However this is undesirable, as the alignment of `tail` is unknown (the current PR assumes an alignment of 1). Unfortunately we can't prevent it in the general case as the tail could be a type parameter :
```rust
#[repr(C)]
struct FfiStruct<T: ?Sized> {
    data: u8,
    more_data: u32,
    tail: T,
}
```

Adding a `DynSized` trait would solve this as well, by requiring tail fields to be bound by it.

- Despite being unsized, pointers to extern types are thin and can be casted from/to integers. However it is not possible to write a `null<T>() -> *const T` function which works with extern types, as I've explained here : #43467 (comment)

- Trait objects cannot be built from extern types. I intend to support it eventually, although how this interacts with `DynSized`/`size_of_val` is still unclear.

- The definition of `c_void` is unmodified

bors added a commit that referenced this issue Oct 28, 2017

Auto merge of #44295 - plietar:extern-types, r=arielb1
Implement RFC 1861: Extern types

A few notes :

- Type parameters are not supported. This was an unresolved question from the RFC. It is not clear how useful this feature is, and how variance should be treated. This can be added in a future PR.

- `size_of_val` / `align_of_val` can be called with extern types, and respectively return 0 and 1. This differs from the RFC, which specified that they should panic, but after discussion with @eddyb on IRC this seems like a better solution.
If/when a `DynSized` trait is added, this will be disallowed statically.

- Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are `!Sync`, `!Send` and `!Freeze`. This seems like the correct behaviour to me.
Manual `unsafe impl Sync for Foo` is still possible.

- This PR allows extern type to be used as the tail of a struct, as described by the RFC :
```rust
extern {
    type OpaqueTail;
}

#[repr(C)]
struct FfiStruct {
    data: u8,
    more_data: u32,
    tail: OpaqueTail,
}
```

However this is undesirable, as the alignment of `tail` is unknown (the current PR assumes an alignment of 1). Unfortunately we can't prevent it in the general case as the tail could be a type parameter :
```rust
#[repr(C)]
struct FfiStruct<T: ?Sized> {
    data: u8,
    more_data: u32,
    tail: T,
}
```

Adding a `DynSized` trait would solve this as well, by requiring tail fields to be bound by it.

- Despite being unsized, pointers to extern types are thin and can be casted from/to integers. However it is not possible to write a `null<T>() -> *const T` function which works with extern types, as I've explained here : #43467 (comment)

- Trait objects cannot be built from extern types. I intend to support it eventually, although how this interacts with `DynSized`/`size_of_val` is still unclear.

- The definition of `c_void` is unmodified
@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Nov 16, 2017

@plietar In #44295 you wrote

Auto traits are not implemented by default, since the contents of extern types is unknown. This means extern types are !Sync, !Send and !Freeze. This seems like the correct behaviour to me. Manual unsafe impl Sync for Foo is still possible.

While it is possible for Sync, Send, UnwindSafe and RefUnwindSafe, doing impl Freeze for Foo is not possible as it is a private trait in libcore. This means it is impossible to convince the compiler that an extern type is cell-free.

Should Freeze be made public (even if #[doc(hidden)])? cc @eddyb #41349.

Or is it possible to declare an extern type is safe-by-default, which opt-out instead of opt-in?

extern {
    #[unsafe_impl_all_auto_traits_by_default]
    type Foo;
}
impl !Send for Foo {}
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Nov 16, 2017

@kennytm What's the usecase? The semantics of extern type are more or less that of a hack being used before the RFC, which is struct Opaque(UnsafeCell<()>);, so the lack of Freeze fits.
That prevents rustc from telling LLVM anything different from what C signatures in clang result in.

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Nov 16, 2017

@eddyb Use case: Trying to see if it's possible to make CStr a thin DST.

I don't see anything related to a cell in #44295? It is reported to LLVM as an i8 similar to str. And the places where librustc_trans involves the Freeze trait reads the real type, not the LLVM type, so LLVM treating all extern type as i8 should be irrelevant?

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Nov 16, 2017

@kennytm So with extern type CStr;, writes through &CStr would be legal, and you don't want that?
The Freeze trait is private because it's used to detect UnsafeCell and not meant to be overriden.

bors added a commit that referenced this issue Nov 16, 2018

Auto merge of #55672 - RalfJung:miri-extern-types, r=eddyb
miri: accept extern types in structs if they are the only field

Fixes #55541

Cc @oli-obk @eddyb #43467

kennytm added a commit to kennytm/rust that referenced this issue Nov 17, 2018

Rollup merge of rust-lang#55672 - RalfJung:miri-extern-types, r=eddyb
miri: accept extern types in structs if they are the only field

Fixes rust-lang#55541

Cc @oli-obk @eddyb rust-lang#43467

bors added a commit that referenced this issue Nov 17, 2018

Auto merge of #55672 - RalfJung:miri-extern-types, r=eddyb
miri: accept extern types in structs if they are the only field

Fixes #55541

Cc @oli-obk @eddyb #43467

bors added a commit that referenced this issue Nov 18, 2018

Auto merge of #55672 - RalfJung:miri-extern-types, r=eddyb
miri: accept extern types in structs if they are the only field

Fixes #55541

Cc @oli-obk @eddyb #43467
@shepmaster

This comment has been minimized.

Copy link
Member

shepmaster commented Jan 5, 2019

Comments from jethrogb and Ericson2314 mention that LLVM uses a special type opaque instead of Rust's current empty type, but I can't find any follow up on it. Rust still uses an empty type:

Rust

1.33.0-nightly 2019-01-04 f381a96

extern {
    type nuffin_t;

    fn myalloc() -> *mut nuffin_t;
    fn myfree(_: *mut nuffin_t);
}
%"::nuffin_t" = type {}

declare %"::nuffin_t"* @myalloc() unnamed_addr #0
declare void @myfree(%"::nuffin_t"*) unnamed_addr #0

C

clang version 8.0.0 (trunk 350447)

struct nuffin_t;

struct nuffin_t *myalloc();
void myfree(struct nuffin_t*);
%struct.nuffin_t = type opaque

declare dso_local void @myfree(%struct.nuffin_t*) #1
declare dso_local %struct.nuffin_t* @myalloc(...) #1
@Ericson2314

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Jan 5, 2019

Good catch!

Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 18, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.

Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019

Rollup merge of rust-lang#56594 - sdroege:c_void-is-not-never, r=TimNN
Remove confusing comment about ideally using `!` for `c_void`

Using `!` for `c_void` would have the problem that pointers and
potentially references to an uninhabited type would be created, and at
least for references this is UB.

In addition document that newtype wrappers around `c_void` can be used
safely in place of `extern type` until the latter is stabilized.

----

I'm not 100% sure about the usage for opaque types as the [nomicon](https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs) still recommends using `#[repr(C)] pub struct Foo { _private: [u8; 0] }` but it seems like these two should be equivalent in the end? Also the `#[repr(C)]` (in both cases) should be unneeded because such types never being passed by value, never being dereferenced but only passed around as pointer or reference, so the representation of (*values* of) the type itself should not matter at all?

Also in context of `c_void` and `!` the second unresolved question in the [`extern type`](rust-lang#43467) stabilization ticket seems relevant

> In [std's](https://github.com/rust-lang/rust/blob/164619a8cfe6d376d25bd3a6a9a5f2856c8de64d/src/libstd/os/raw.rs#L59-L64) source, it is mentioned that LLVM expects i8* for C's void*.
> We'd need to continue to hack this for the two c_voids in std and libc.
> But perhaps this should be done across-the-board for all extern types?
> Somebody should check what Clang does.

Please correct me if my understanding is wrong and everything's actually fine as is.
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Feb 13, 2019

@shepmaster I independently came across that (well, @RalfJung pointed it out) and I opened #58271 last week, which I'm not sure how to fix.

If what I did is different from type opaque, that might be why it fails.

But also, note that type {} is just a tuple with no fields, i.e. LLVM's (), not an "empty type" in the Rust sense of enum Void {}.

@raphaelcohn

This comment has been minimized.

Copy link

raphaelcohn commented Mar 6, 2019

I've been trying to use extern type to model some variably-sized data that needs to co-operate with a C API.

I've come across an issue where I'd like to define a method which involves casting from various raw pointers, come of which are Sized and some of which are extern type. In trying to make the code common, I need to use T: ?Sized, but that then creates a problem, as it 'pulls in' all types, including fat pointers, which I don't want (the underlying C code assumes that all pointers are usize in size).

Is there any way to use the type system to differentiate extern type from just ?Sized (similar problems have bit me in the past - it would be nice to use the type system to restrict based on sizes or alignment patterns or repr(u16) or things being enums with specific layouts - but ho-hum).

@alexreg

This comment has been minimized.

Copy link
Contributor

alexreg commented Mar 6, 2019

@raphaelcohn I believe the plan is to add a DynSized, so you could use T: DynSized to include basically all types except extern types. See rust-lang/rfcs#2255 for more info. I'm not the most knowledgeable about this though.

@jethrogb

This comment has been minimized.

Copy link
Contributor

jethrogb commented Mar 6, 2019

@alexreg yeah so that's the opposite of what @raphaelcohn wants I think :)

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 6, 2019

rust-lang/rfcs#2580 is another proposal in somewhat the same space as DynSized. With that implemented, you could use T: ?Sized + Pointee<Metadata=()> to restrict a type parameter to types that have thin pointers pointing to them, including extern types.

@alexreg

This comment has been minimized.

Copy link
Contributor

alexreg commented Mar 7, 2019

@jethrogb It's exactly what he wants, the way I read it. Or maybe he needs both. Regardless, negative bounds for auto traits exist, so e.g. !DynSized would work.

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Mar 7, 2019

@alexreg You could write impl !Send for Stuff {} but that doesn't mean where Stuff: !Send is a valid bound.

@raphaelcohn

This comment has been minimized.

Copy link

raphaelcohn commented Mar 7, 2019

@alexreg, @jethrogb @SimonSapin @kennytm Thank you all.
Essentially, I need a bound that says T: thin pointers. I think @SimonSapin's solution T: ?Sized + Pointee<Metadata=()> would work, but it does seem rather non-intuitive.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 7, 2019

Indeed, the RFC also proposes a trait alias pub trait Thin = Pointee<Metadata=()>; in the std::ptr module. But at this point it’s only a proposal.

@alexreg

This comment has been minimized.

Copy link
Contributor

alexreg commented Mar 7, 2019

@kennytm It's not? Well, it needs to be!

@Ericson2314

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Mar 7, 2019

For the record, I think we need Pointee and DynSized. Thing + ?Sized still means you are responsible for coming up with the dynamic size and allocation from a (), which means panicking, which is no good. Likewise ?DynSized without Thin means you can do unsafe operations, but have no means to ensure the ABI is correct, so generic FFI stuff cannot be written. Both is wonderful, and will making custom DSTs so much easier to discuss because the unnecessary degrees of freedom don't exist.

@raphaelcohn

This comment has been minimized.

Copy link

raphaelcohn commented Mar 8, 2019

And anything that lets me safely coerce a slice to an iovec would be most welcome...

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.