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
Closed

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

dgrunwald opened this issue Jan 6, 2015 · 10 comments
Labels
A-FFI Area: Foreign function interface (FFI) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@dgrunwald
Copy link
Contributor

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.

@kmcallister kmcallister added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-FFI Area: Foreign function interface (FFI) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Jan 6, 2015
@cactorium
Copy link
Contributor

Mind if I try to fix this?

@dgrunwald
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

Ack, sorry, what's a FFI position?

@dylanmckay
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
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
Copy link
Member

Yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

7 participants