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

Should extern "C" include "C" #52

Closed
nrc opened this issue Jan 18, 2017 · 13 comments
Closed

Should extern "C" include "C" #52

nrc opened this issue Jan 18, 2017 · 13 comments

Comments

@nrc
Copy link
Member

nrc commented Jan 18, 2017

I.e., should we format extern "C" fn as that or extern fn?

Previous discussion: rust-lang/rustfmt#451

@strega-nil
Copy link

I use extern fn near exclusively, personally.

@steveklabnik
Copy link
Member

I was told in review once that extern fn was preferred, so it's what I use.

@solson
Copy link
Member

solson commented Jan 18, 2017

+1 for the abbreviated version. We should either use it or deprecate it, and I've seen no signs of a plan to deprecate.

@nrc
Copy link
Member Author

nrc commented Jan 18, 2017

We should either use it or deprecate it

There's been a very high bar for this, generally, so even if it were disapproved of, it would be unlikely to be deprecated.

@solson
Copy link
Member

solson commented Jan 18, 2017

Good point. I guess I actually mean I haven't encountered disapproval of it before.

Although, if we decided to go the explicit route here, that suggests a value judgement that extern fn is too confusing and we should stop ever using it, which is de facto deprecation...

@joshtriplett
Copy link
Member

joshtriplett commented Jan 18, 2017

I'd prefer extern fn for the one-line case. For a block of functions, extern "C" { seems fine; since that would appear on a line of its own, the horizontal space usage seems fine.

@petrochenkov
Copy link

petrochenkov commented Jan 19, 2017

I always use extern "ABI" because the syntax is silly to start with.
"C" is the important part of this modifier and there's nothing "extern" in function with body and non-Rust ABI. I wish it was abi "C" fn() and extern abi "C" { ... } without default ABI, but the ship has sailed long ago.

@japaric
Copy link
Member

japaric commented Jan 24, 2017

I have the strong opinion that we should be explicit here. Because:

  • It makes the code self documenting. Or at least it better expresses the intention of the programmer.
extern "C" {
    fn foo();
}

This (most likely) indicates that foo comes from a C library.

pub extern "C" fn bar() { .. }

This (most likely) indicates that bar will be used by a C library.

extern "Rust" {
    fn main();
}

Whereas, this indicates that main comes from a Rust crate.

  • "C" is not the only ABI that Rust has but seeing extern fn everywhere (as "C" is the most common non-Rust ABI) could make people believe that's the only way to use extern.

For example, the Rust port of compiler-rt, the compiler-builtins crate, has to use the "aapcs" ABI for the aeabi intrinsics because that's the calling convention that LLVM uses. Every other symbol has to use the "C" ABI.

pub extern "aapcs" fn __aeabi_memcpy(..) { .. }
pub extern "C" fn __floatundidf(..) { .. }

Getting the ABI wrong, e.g. using just extern fn everywhere, can lead to nasty ABI mismatch bugs.

  • It would hopefully be didactic for beginners. Upon their first encounter with a extern "C" block / fn, they may ask themselves "Wait, can "C" be a different string?", then they'll check the documentation and learn about the other ABIs, specially about the ones that are not about FFI like "msp430-interrupt" and "ptx-kernel".

I wish it was abi "C" fn() and extern abi "C" { ... } without default ABI, but the ship has sailed long ago.

I agree 100%.

@nrc
Copy link
Member Author

nrc commented Feb 9, 2017

I would like to move to FCP here with an inclination towards being explicit. I think @japaric makes a great case above and we need to choose one way or the other.

@solson
Copy link
Member

solson commented Feb 9, 2017

I am also convinced by @japaric's arguments.

Getting the ABI wrong, e.g. using just extern fn everywhere, can lead to nasty ABI mismatch bugs.

If this is really so dangerous, we should not just stop using extern fn in rustfmt but also lint against it (maybe in clippy?). I would suggest a lint in rustc but it would probably annoy too broad a group of people.

@nrc
Copy link
Member Author

nrc commented Mar 6, 2017

Seems we have consensus and no further comments

@dns2utf8
Copy link

dns2utf8 commented Mar 8, 2017

So do I get this right? Rust will go the more expressiv way with extern "C" fn?

@nrc
Copy link
Member Author

nrc commented Mar 14, 2017

We've changed our process: rather than requiring a full RFC, a PR to close this issue only requires making the changes discussed here to the style guide. You can see more details about the process in the readme for this repo.

If you'd like to help with this work, let me know, I'd be very happy to help you help us.

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 15, 2017
…umeGomez

rustdoc: Display `extern "C" fn` instead of `extern fn`

It was decided in rust-lang/style-team#52 to be explicit about the ABI so rustdoc should follow suit.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017
…umeGomez

rustdoc: Display `extern "C" fn` instead of `extern fn`

It was decided in rust-lang/style-team#52 to be explicit about the ABI so rustdoc should follow suit.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 16, 2017
…umeGomez

rustdoc: Display `extern "C" fn` instead of `extern fn`

It was decided in rust-lang/style-team#52 to be explicit about the ABI so rustdoc should follow suit.
@nrc nrc closed this as completed in #98 Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants