Skip to content

Empty structs should not need #[repr(C)] #18686

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

Closed
cgaebel opened this issue Nov 6, 2014 · 11 comments
Closed

Empty structs should not need #[repr(C)] #18686

cgaebel opened this issue Nov 6, 2014 · 11 comments

Comments

@cgaebel
Copy link
Contributor

cgaebel commented Nov 6, 2014

In a lot of FFI code, I see patterns like:

struct X {}

fn some_c_function(this: *mut X);

where X is used as an opaque "black-box" object. With rust today, X needs a #[repr(C)]. That's not really necessary, and caused a bunch of noisy warnings when updating servo's rust version.

Can someone mark this bug as easy so a beginner can use it to get started?

@ghost
Copy link

ghost commented Nov 6, 2014

The explicit #[repr(C)] is still desired on empty structs, I'd say. For example, implementing Drop on an empty struct changes the representation by adding a drop flag. Now, currently implementing Drop on an empty struct with #[repr(C)] is not rejected by the compiler but that is tracked separately in #18380. And, of course, dynamic drop flags embedded within types' representations are going away but the point of #[repr(C)] is to leave the doors open for future representation changes.

@cgaebel
Copy link
Contributor Author

cgaebel commented Nov 6, 2014

Regardless of the representation of the struct on the rust side, it should not break C code that requires a struct with no fields, since it won't be accessing any of those fields.

Even when passing one of these things by-value to C, caller cleans up the stack with cdecl, so it should be fine. And in the more frequent case of getting a X* from C and passing it to other C functions, this warning is definitely just noise.

@mahkoh
Copy link
Contributor

mahkoh commented Nov 6, 2014

struct X {
};

struct Y {
    struct X x;
    int i;
};

@cgaebel
Copy link
Contributor Author

cgaebel commented Nov 6, 2014

After more investigation, empty structs are not valid C99. Therefore, it seems silly to mark it as #[repr(C)].

@mahkoh
Copy link
Contributor

mahkoh commented Nov 6, 2014

Irrelevant as long as GCC allows them.

@cgaebel
Copy link
Contributor Author

cgaebel commented Nov 6, 2014

If it's not valid C, I see (heh) no reason for rust to support it with #[repr(C)].

@mahkoh
Copy link
Contributor

mahkoh commented Nov 6, 2014

Meanwhile outside the ivory tower we also want to interact with C code that doesn't only use ISO C features (correctly.) Think: 99% of all C code.

@soltanmm
Copy link

soltanmm commented Nov 6, 2014

At the risk of entering an argument I may not understand fully... The 'ivory tower' counter-argument here given by mahkoh seems to be placing GCC in an ivory tower of its own. If the standard doesn't support empty structs, why should the 'standard' (reference?) for Rust condone something non-standard? Why not leave that to implementation defined behavior, in the same way that anything done in the C stdlib that may be IB is automatically IB for Rust linking to the C stdlib?

I'unno, transitive property of implementation-defined-ness, or something.

@emberian
Copy link
Member

emberian commented Nov 7, 2014

@cgaebel When I implemented this feature, I struggled a bit with what to do about empty structs. I ended up going with this because I think it is annoying and grating to add a field to a struct and suddenly need #[repr(C)] on it. It is possible to special-case "pointer to an empty struct" in the lint.

@cgaebel
Copy link
Contributor Author

cgaebel commented Nov 7, 2014

I like the idea of special-casing pointer-to-an-empty-struct.

@steveklabnik
Copy link
Member

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants