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

improper_ctypes: also check extern fn's #30499

Closed
wants to merge 7 commits into from

Conversation

TimNN
Copy link
Contributor

@TimNN TimNN commented Dec 20, 2015

The improper ctypes lint currently only checks functions inside of extern blocks. This changes the lint so that it also checks all functions with an extern calling convention.

Closes #19834.

This probably need a crater run.

Edit: I just realised that there's still some stuff to be tested: generics and methods, I will add those soon.

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@TimNN TimNN force-pushed the ctypes-extern-fns branch from 6e6ac2e to 78462fe Compare December 20, 2015 17:30
@nagisa
Copy link
Member

nagisa commented Dec 20, 2015

IMO this is a wrong thing to do or at very least needs an RFC process to happen. That is because use of a certain calling convention for functions does not necessarily mean they are meant to be used via FFI.

@TimNN
Copy link
Contributor Author

TimNN commented Dec 20, 2015

I understand your argument and it is the same reason why structs with #[repr(C)] do not trigger a warning by themselves as far as I know.

On the other hand we have people unfamiliar with rust who would have benefited from such a lint (see #19834 (comment)) and I personally would like for rust to check that I did not make a mistake in my c api.

I have rarely seen a use for functions with a custom calling convention which were not meant to be used from c, so in my mind warning about them is preferable, especially since you can opt-out of these checks but not opt-in.

I see now that the situation is more controversial than I though so I'm going to leave this open for a few days, to wait for some more comments, and then, depending on the response, write a RFC first.

Maybe the best solution would be to have three separate lints, one for checking extern blocks (the current improper_ctypes, one for checking all functions with a non-rust calling convention and one for all types with #[repr(C)].

@TimNN
Copy link
Contributor Author

TimNN commented Dec 26, 2015

I've been think some more about this and came to the conclusion that if one can argue that there are use cases for extern fns with improper ctypes than there are use cases for extern blocks with improper ctypes.

As such I decided to close this pull request for now and instead am going to write an RFC (or open an RFC issue) soon.

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

Successfully merging this pull request may close these issues.

4 participants