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

Using #[repr(C)] on unit structs should warn #20660

Closed
dgrunwald opened this Issue Jan 6, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@dgrunwald
Copy link
Contributor

dgrunwald commented Jan 6, 2015

The C language does not allow empty structs.
gcc and clang allow them as a language extension, and they have size 0 there. With --pedantic, both will emit a warning on empty C structs.
msvc refuses to compile C code using empty structs.

In C++, empty structs are allowed -- but there, they have size 1!

In rust, a #[repr(C)] unit struct is given size 0. This makes it compatible with gcc and clang in C mode; but has the potential to cause trouble when interfacing with a C++ library.

I think the improper_ctypes lint should issue a warning when #[repr(C)] is applied to a size 0 struct.
Users can suppress the warning when interfacing with C code using the gcc extensions.

@cactorium

This comment has been minimized.

Copy link
Contributor

cactorium commented Jan 6, 2015

Mind if I try to fix this?

@dgrunwald

This comment has been minimized.

Copy link
Contributor Author

dgrunwald commented Jan 6, 2015

I don't mind.
I currently don't plan to start hacking on rustc, though that might change in the future. (I have some experience doing compilers)

@tomjakubowski

This comment has been minimized.

Copy link
Contributor

tomjakubowski commented Jan 7, 2015

In general, #[repr(C)] can be added to any type whether or not it can actually be safely represented as a C type. It would be easier to add the warning if such a 0-sized type is used in an FFI position, which ought to be a simple addition to the improper_ctypes lint.

By the way, I think it would be dangerous to assume that #[repr(C)] struct Foo; is compatible with the GCC extensions.

@cactorium

This comment has been minimized.

Copy link
Contributor

cactorium commented Jan 7, 2015

Ack, sorry, what's a FFI position?

@dylanmckay

This comment has been minimized.

Copy link
Contributor

dylanmckay commented Jan 7, 2015

Ack, sorry, what's a FFI position?

tomjakubowski means that a warning should be issued in the case that a unit struct is used in FFI code (for example, in a call to an external C function)

@cactorium

This comment has been minimized.

Copy link
Contributor

cactorium commented Jan 7, 2015

Ah, that make sense. The improper_ctypes lint code already limits what it checks to exactly that, so I've managed to add a check for unit structs only in that case by following the path of least resistance. Pull request soon hopefully!

@dgrunwald

This comment has been minimized.

Copy link
Contributor Author

dgrunwald commented Jan 8, 2015

I forgot that there is another possible use of unit structs in FFI code: representing incomplete types.
These don't actually have size 0 on the C side of things, but rather an unknown size. We can't use DST in FFI code to represent these because we don't want fat pointers, so the usual solution is to use a unit struct.
Because pointers to incomplete types are never dereferenced in Rust code, the representation chosen by Rust doesn't matter -- we could even skip checking for #[repr(C)].

Problems occur only when a unit struct is passed by value, or if we pass a pointer to a struct that contains a unit struct.
So I think the solution is to not recurse into pointer types if the target type is a unit struct.

@cactorium

This comment has been minimized.

Copy link
Contributor

cactorium commented Jan 11, 2015

Sorry for taking so long to reply, it's taking a while to figure out how to do this. We can't not recurse, since we need to check for structs inside structs that have unit structs as fields, so I've been reading a lot of the AST and type tree source to figure out how to tell a pointer to a struct from a struct and how they're all represented in the trees, which has been oddly difficult. Getting there though!

@TimNN

This comment has been minimized.

Copy link
Contributor

TimNN commented Dec 20, 2015

When a zero-sized struct is used in an extern block the improper_ctypes lint triggers in current stable, so this can probably be closed: http://is.gd/VLsUnr

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 20, 2015

Yay!

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.