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

extern "C" functions returning ZSTs #115091

Open
ChayimFriedman2 opened this issue Aug 22, 2023 · 9 comments
Open

extern "C" functions returning ZSTs #115091

ChayimFriedman2 opened this issue Aug 22, 2023 · 9 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@ChayimFriedman2
Copy link
Contributor

Currently, for extern "C" functions returning ZSTs the compiler returns void (as well as for other ABIs, but this is not related). However, improper_ctypes_definitions fire for such functions (if they don't return ()), e.g.:

// #[repr(C)] // Adding that does not change the outcome.
pub struct Zst;

pub extern "C" fn foo() -> Zst {
    Zst
}
warning: `extern` fn uses type `Zst`, which is not FFI-safe
 --> src/lib.rs:4:28
  |
4 | pub extern "C" fn foo() -> Zst {
  |                            ^^^ not FFI-safe
  |
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout
note: the type is defined here
 --> src/lib.rs:2:1
  |
2 | pub struct Zst;
  | ^^^^^^^^^^^^^^
  = note: `#[warn(improper_ctypes_definitions)]` on by default

Playground.

It's pretty clear we do not want to guarantee the FFI-safety of ZST arguments. But what about the FFI safety of ZST return types? Do we want to guarantee they'll keep translating to void, or keep this not guaranteed?

Context: https://stackoverflow.com/q/76950446/7884305.

@rustbot label T-lang

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 22, 2023
@Noratrieb Noratrieb added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 22, 2023
@Noratrieb
Copy link
Member

note that questions like these will see more activity if asked on https://internals.rust-lang.org/ on zulip

@Noratrieb
Copy link
Member

But something worth pointing out is that something being a ZST is not necessarily part of its public API, so using ZSTs with private fields from other crates in return position is not okay because they could be changed to not be ZSTs in the future.

@ShE3py
Copy link
Contributor

ShE3py commented Aug 22, 2023

Some types may be optimized to ZSTs ;

pub enum Errno {
    Generic
}

pub extern "C" fn mainloop() -> Result<!, Errno> {
    Err(Errno::Generic)
}

so the generated glue will be

void mainloop();

which is kinda not good.


I think for FFI, users should use a type alias:

type IfThisReturnsItHasErrored = ();

As the type's meaning shouldn't be dropped, and the generated glue will be better:

typedef void IfThisReturnsItHasErrored;

IfThisReturnsItHasErrored mainloop();

@ChayimFriedman2
Copy link
Contributor Author

@ShE3py But sometimes as in the linked Stack Overflow question, people want a proper type with privacy and maybe generics. And I agree we should not allow all types that are ZSTs, especially types we do not guarantee are ZSTs (like the enum), but for types that we do guarantee they will be ZSTs I think it makes sense to guarantee that returning them is equal to returning void.

@ShE3py
Copy link
Contributor

ShE3py commented Aug 22, 2023

This seems to works:

use std::marker::PhantomData;

#[repr(C)]
pub struct Invariant<'a> {
    _priv: (),
    _phantom: PhantomData<fn(&'a ()) -> &'a ()>
}

#[forbid(improper_ctypes_definitions)]
pub extern "C" fn foo<'a>() -> Invariant<'a> {
    Invariant { _priv: (), _phantom: PhantomData }
}
example::foo:
        ret

I think it denies #[repr(transparent)] as it's not FFI-related, but using #[repr(C)] is asking the struct to be FFI-safe.

@Noratrieb
Copy link
Member

#[repr(transparent)] is FFI-safe if the inner type is FFI-safe.

@markcsaving
Copy link

@ShE3py The issue with using a #[repr(C)] struct is that the extern "C" ABI might do special treatment for structs, so the corresponding return type on the C side might not be void. There don't seem to be any guarantees about this.

@markcsaving
Copy link

@Nilstrieb I don't think your point applies here. If we have

#[repr(transparent)]
struct TwoZSTFields((), PhantomData<()>);

which field is "the inner type"?

The documentation seems to suggest that using #[repr(transparent)] in this case isn't allowed at all, even though the compiler currently lets us do it. Checking the documentation for #[repr(transparent)], it states explicitly in the Rustonomicon that

#[repr(transparent)] can only be used on a struct or single-variant enum that has a single non-zero-sized field (there may be additional zero-sized fields).

In the Rust Reference, the wording is similar; there must be a single non-zero-sized field. There is an open pull request on this topic here which seems to be in limbo.

@kamirr
Copy link
Contributor

kamirr commented Jan 8, 2024

Some types may be optimized to ZSTs ;

pub enum Errno {
    Generic
}

pub extern "C" fn mainloop() -> Result<!, Errno> {
    Err(Errno::Generic)
}

so the generated glue will be

void mainloop();

which is kinda not good.

@ShE3py This code is invalid to begin with. Errno is not repr(C), and it has to be in order to be a valid return type of an extern "C" function. If you mark it as repr(C) or repr(u*) then the layout becomes guaranteed and the concern of optimization to ZST is void (pardon the pun).

But something worth pointing out is that something being a ZST is not necessarily part of its public API, so using ZSTs with private fields from other crates in return position is not okay because they could be changed to not be ZSTs in the future.

@Nilstrieb API and ABI stability are separate concerns. Yes, a type can stop being a ZST and it's ABI would change from that of void to that of another type. If you have a struct S { a: u32 } and change it to S { a: u32, b: u32 } then you break the ABI as well whilst maintaining the public API (note that the fields are private), starting with a ZST is not relevant.

@workingjubilee workingjubilee added A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants