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

Disallow omitting the ABI in extern declarations #697

Closed
wants to merge 2 commits into from

Conversation

ranma42
Copy link
Contributor

@ranma42 ranma42 commented Jan 21, 2015

@tomjakubowski
Copy link
Contributor

The RFC should clarify whether this applies to "extern functions" (extern "C" fn foo() { ... }) as well as "external blocks" (extern "C" { fn foo(); fn bar(c_int); }).

@comex
Copy link

comex commented Jan 22, 2015

I guess it is nice to be explicit about it, considering that in C declaring a function extern does absolutely nothing, so people might instinctively overlook it.

@ranma42 ranma42 changed the title Disallow omitting the ABI in extern blocks Disallow omitting the ABI in extern declarations Jan 22, 2015
@ranma42
Copy link
Contributor Author

ranma42 commented Jan 22, 2015

@tomjakubowski I meant to propose the change for both functions and blocks. I tried to modify the text of the RFC to make this more clear and to explain why I think that doing it for just one of them is a bad idea.

@nikomatsakis
Copy link
Contributor

I still find the default fairly reasonable.

@huonw
Copy link
Member

huonw commented Jan 22, 2015

I'm not personally much in favour of this, extern fn feels "smoother"/more natural than extern "C" fn, but I'm not overly worried either way.

@ranma42
Copy link
Contributor Author

ranma42 commented Jan 23, 2015

@nikomatsakis If any default should be, I agree that the current one is very reasonable.

@huonw extern fn might feel better if taken by itself, but in a context in which you might also have other extern declarations it would not be as smooth. Compare

#[cfg(mycfg1)]
pub extern fn foo(bar: i32) { }

#[cfg(mycfg2)]
pub extern "stdcall" fn foo(bar: i32) { }

#[cfg(mycfg3)]
pub extern "fastcall" fn foo(bar: i32) { }

with

#[cfg(mycfg1)]
pub extern "C" fn foo(bar: i32) { }

#[cfg(mycfg2)]
pub extern "stdcall" fn foo(bar: i32) { }

#[cfg(mycfg3)]
pub extern "fastcall" fn foo(bar: i32) { }

The second one looks much better to me. I know that you could explicitly mention the "C" ABI specifier in such a case and omit it in other ones, but overall this would make the code less consistent.

I feel like the ABI specifier makes it easier to understand what the extern keyword means, i.e. the function I'm declaring has a specific ABI.

@Ericson2314
Copy link
Contributor

I'm somewhat mystified why we use extern both to specify ABIs and to specify linkage. Seems like a wart inherited from C. The current rules regarding extern-blocks vs stand-alone items, unsafety, functions vs statics, seem arbitrary.

@Ericson2314
Copy link
Contributor

A concrete proposal:

ABI_DECL ::= ('abi' ABI)

FN_TYPE ::= ABI_DECL? ('for' '<' LIFETIME* '>')? fn '(' TYPE* ')' '->' Type

FN_SIG ::= ABI_DECL? ('for' '<' LIFETIME* '>')? fn IDENTIFIER '(' TYPE* ')' '->' Type

PARTIAL_FN_DECL ::= FN_SIG ';'
FN_DECL ::= 'extern' PARTIAL_FN_DECL


PARTIAL_STATIC_DECL ::= 'static' IDENTIFER ':' TYPE
STATIC_DECL ::= 'extern' PARTIAL_STATIC_DECL ';'
             | PARTIAL_STATIC_DECL '=' EXPR ';'


EXTERN_BLOCK ::= 'extern' ABI_DECL? '{' (PARTIAL_FN_DECL | PARTIAL_STATIC_DECL ';')* '}'

ITEM ::= FN_DECL | STATIC_DECL | EXTERN_BLOCK | ...

semantics:

  • abi "...": declares the abi, nothing else.
  • extern: implies unsafe, undefined symbol, and, optionally, abi "C" unless overridden with explicit abi "...".
  • extern ?? { .. }: prepend extern to items within, and interpret with above rules. If abi is defined on inner item and whole block, the inner item abi takes precedence.

Finally, an extern block with explicit abi, and only statics inside triggers a warning. (statics don't care about abi.)

@brendanzab
Copy link
Member

I'm in favor of explicitness - I prefer to be explicit with ABIs in my own code anyway. "C" is a good signal to the reader they are dealing with a foreign ABI.

@ranma42
Copy link
Contributor Author

ranma42 commented Jan 26, 2015

@Ericson2314 my understanding is that linkage and ABI are already independent: the first is controlled by the #[linkage = "..."] annotation and possibly by pub, the second one by extern "..."

Do you mean that it might be nice to separate the extern keyword used for ABI control from linkage dependencies expressed using extern crate?

@Ericson2314
Copy link
Contributor

@ranma42 Interesting, I did not know about #[linkage = "..."]. Unfortunately, extern blocks at least effect linkage as seen here: http://doc.rust-lang.org/src/core/panicking.rs.html#55.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2015

-1. The default "C" abi really makes sense. If someone like explicit, just
always write it.

@ranma42
Copy link
Contributor Author

ranma42 commented Jan 27, 2015

@Ericson2314 I had misunderstood what you meant for linkage. My current understanding is that you mean that foreign functions are special and are only allowed in extern blocks. Is this correct?
I am not sure if this is a problem and if it is actually intertwined with the change I'm advocating.

@Ericson2314
Copy link
Contributor

@ranma42 Ah ok. I was mistaken, the situation is less bad than I thought in that extern blocks are the sole way to declare undefined items. I still think using the same keyword for ABI and extern blocks is confusing -- it certainly confused me.

So yes, while I now see that what I am complaining about can be fixed independently from your RFC (sorry about that), the fixes complement each other. Consider this:

MODIFIERS ::= 'unsafe'? ('abi' ABI)?

FN_TYPE ::= MODIFIERS ('for' '<' LIFETIME* '>')? fn '(' TYPE* ')' '->' Type

FN_SIG ::= MODIFIERS ('for' '<' LIFETIME* '>')? fn IDENTIFIER '(' TYPE* ')' '->' Type
FN_DECL ::= FN_SIG ';'

STATIC_SIG ::= 'static' IDENTIFER ':' TYPE
STATIC_DECL ::= STATIC_SIG ';'

EXTERN_BLOCK ::= 'extern' MODIFIERS '{' (FN_DECL | STATIC_DECL)* '}'

ITEM ::= EXTERN_BLOCK | FN_SIG BLOCK | STATIC_SIG '=' EXPR ';' | ...

Semantics:

  • abi "...": declares the abi, nothing else.
  • extern ?? { .. }: allows for undefined, externally linked items. extern { .. } does not imply anything regarding unsafety or abi, but the modifiers specified on the block will apply to all functions declared within (unless overridden).

@Ericson2314
Copy link
Contributor

Also, adding an abi keyword allows the compromise where (extern blocks with no modifiers) still imply unsafe abi "C", but function types and sigs cannot elide the "C". i.e. they must be abi "C" fn ..., not abi fn ...

@brson brson self-assigned this Jan 29, 2015
@ranma42
Copy link
Contributor Author

ranma42 commented Feb 16, 2015

@brson Should we try and agree on a resolution about this in time for alpha2? This would certainly be a breaking-change (even though a minor one or at least easy to fix), so it might not be appropriate for the beta (AFAIU the language syntax should not be modified in an incompatible way after alpha).

@ms-ati
Copy link

ms-ati commented May 15, 2015

Is this bikeshedding?

(just having a laugh)

@aturon aturon added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jun 1, 2015
@nikomatsakis
Copy link
Contributor

It's too late to make this change without breaking backwards compatibility, and support was mixed in any case, so I'm going to close this RFC. Thanks for the suggestion, @ranma42!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.