Add extern "custom"#3980
Conversation
|
Thanks @folkertdev for putting this together. @rfcbot fcp merge lang |
|
@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
@rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
@rfcbot reviewed |
|
Seeing a RFC entering a final comment period 1 hour after it was opened is extremely rare, and personally makes me worried. Even if it was discussed extensively in the past and all relevant team members are on-board, I believe we should let the community more time to discuss it. |
|
imo it's fine since that's what the final comment period is for, announcing and giving people time to look at something before it's accepted, when the team already thinks it's good enough. |
|
Right, this is the (final) ping for concerns to be raised. This was coordinated during the T-lang triage meeting. This feature is based on the following lang proposal, it is basically unchanged from that thread. |
|
I also felt a bit concerned by the sudden FCP, but the methodology here seems pretty canonical since it uses naked functions. The only potential concern is bikeshedding the name "custom" which could potentially be worded as "none" (since this effectively is no ABI) but that's a pretty weak point and not worth blocking IMHO. Plus, it could be changed even post-acceptance but pre-stabilisation. I see no reason to slow down on the FCP process. |
I would expect this time frame to be fairly short in this case. |
|
"none" is incorrect, as explained in the RFC, there is an ABI happening, it's just not an ABI the compiler knows how to use without assistance. |
Yes, this is another reason justifying the choice, although I do think that "none" that the compiler knows about is still appropriate: the compiler is not using any ABI for the function, even though yes, technically, ABI will always exist no matter what. Even just jumping to a label is still an ABI, after all. My point is that this is technically a weak point of contention, but not one I think is worth discussing here, since the name is more than adequately justified. |
Co-authored-by: Trevor Gross <tg@trevorgross.com>
| /// - Expects the dividend and the divisor in r0 and r1. | ||
| /// - Returns the quotient in r0 and the remainder in r1. | ||
| #[unsafe(naked)] | ||
| pub unsafe extern "custom" fn __aeabi_uidivmod() { |
There was a problem hiding this comment.
| pub unsafe extern "custom" fn __aeabi_uidivmod() { | |
| pub unsafe extern "custom" fn __aeabi_uidivmod { |
Since there's no parameter list, and you can't use a call expression with an extern "custom" function, arguably there should be no parentheses in the declaration. Not sure it's worth changing the language grammar for this, though.
Another option is to use the variadic dots syntax:
| pub unsafe extern "custom" fn __aeabi_uidivmod() { | |
| pub unsafe extern "custom" fn __aeabi_uidivmod(...) { |
There was a problem hiding this comment.
removing the () breaks the Function grammar, absolutely not worth it for a pretty niche feature.
making it (...) could work, though i like #3980 (comment) more.
There was a problem hiding this comment.
-1 for removing the parens entirely, seems not worth changing the grammar for.
+0.5 for ..., but I'd prefer just allowing arguments and return types for documentation reasons.
There was a problem hiding this comment.
Variadic arguments are passed differently from regular arguments on some calling conventions though.
There was a problem hiding this comment.
Given that this is a niche feature, that almost nobody will see much less use, I see no reason to spend any syntax budget here.
it's funny/frustrating how so many RFC threads devolve into discussions about syntax: everyone can have an opinion about syntax. The bar for adding syntax is, very deliberately, extremely high.
There was a problem hiding this comment.
@folkertdev it seems to be human nature or sth. Wadler's law in full effect 😄
There was a problem hiding this comment.
Are extern "custom" function pointers allowed? The current implementation on nightly considers unsafe extern "custom" fn(), extern "custom" fn(), and extern "custom" fn(i32) -> u64 to all be valid types.
One option would be to have a single bare extern "custom" function pointer type, to represent a function with known address but unknown calling convention. All other function pointers would be able to coerce to it.
There was a problem hiding this comment.
Good catch! IMHO, it would make sense if you can only define function items as extern "custom" when they're unsafe, argument-less and returning () (or !), to apply the same kind of restrictions on what fn pointer types are usable.
| "add sp, sp, #4", | ||
| "pop {{pc}}", | ||
| trampoline = sym crate::arm::__udivmodsi4 | ||
| ); |
There was a problem hiding this comment.
Would it be worth it to allow the naked_asm! blocks in extern "custom" functions to specify clobbered registers? This could be useful for documentation.
If we allow an explicit parameter list as suggested in #3980, perhaps we could even allow specifying inputs and outputs that reference those parameters (and also the return place?)
|
We talked in today's @rust-lang/lang meeting about function signatures on I proposed, and others on lang agreed, that any signature should be allowed, and used in type checking of custom function pointers. For instance, if you have some hardware table that wants a function pointer to call, you may want to define a signature for that, to make it easier to catch cases where you used the wrong kind of custom function. You write the signature on the function pointer type in the data structure or similar, you then need to match that signature on the |
There was a problem hiding this comment.
I’m looking at RFC 2972 constrained_naked - specifically the future possibilities section - and I think that it’s food for thought on this design. Here’s a quote of that section:
Future possibilities
It would be possible to define new calling conventions that can be used with naked functions.
A previous version of this document defined an
extern "custom"calling convention. It was observed in conversation that calling conventions are really a type and that it could be useful to have calling conventions as part of the type system. In the interest of moving forward with constrained naked functions, it is best to limit the scope of this RFC and defer this (very good) conversation to a future RFC. As a simple workaround, naked functions which do not conform to their specified calling convention should be marked as unsafe and the caller requirements should be documented in the safety section of the documentation per standard convention.It may also be possible to loosen the definition of a naked function in a future RFC. For example, it might be possible to allow the use of some additional, possibly new, operands to the
asm!()block.
In particular the middle part
It was observed in conversation that calling conventions are really a type and that it could be useful to have calling conventions as part of the type system. In the interest of moving forward with constrained naked functions, it is best to limit the scope of this RFC and defer this (very good) conversation to a future RFC. As a simple workaround, naked functions which do not conform to their specified calling convention should be marked as unsafe and the caller requirements should be documented in the safety section of the documentation per standard convention.
seems very relevant IMHO to the discussion of allowing arguments, or allowing non-unsafe signatures to the function. I get the argument that there’s a distinction between cases where it’s just the ABI that’s “special”, but other than that the function is safe to call – from the cases where the function has additional safety predicates.
But I also don’t think this is really how Rust works. If there are still manually-upheld preconditions (not expressed in the type signature) then it simply must be unsafe; no compromises on that: The ABI being unspecified still means that the whole thing is inherently unsafe to use.
I believe that to really allow a "custom" ABI function (and consequently also function pointer type) to call itself truly safe (i.e. non-unsafe), it would need to indicate the ABI at the type level somehow.
I also believe that allowing function arguments would - practically - already allow a way to do this. People could establish a convention where the first argument to a extern "custom" fn(…) is actually a marker type that identifies the actual ABI. If you define your own ABI (by “define”, here I just mean documenting it, in the docs for the MyAbi marker type itself), then function pointers like
let f: extern "custom" fn(MyAbi, x: u32, y: u32) -> u32;could be sensibly be considered fully safe. They could be safe in the sense that you can then provide a sound way to call them from safe code, e.g. as a callback:
fn call_my_abi(callback: extern "custom" fn(MyAbi, u32, u32) -> u32, x: u32, y: u32) -> u32 { … }
fn main() {
let f: extern "custom" fn(MyAbi, x: u32, y: u32) -> u32 = todo!();
let n: u32 = call_my_abi(f, 42, 123); // yay, no unsafe!
}I also think that establishing such a convention is somewhat questionable for downstream users to do (without any official “blessing” of the practice), since different conventions could be incompatible.1 Furthermore, if we ever wanted to have a different, more “dedicated” mechanism for specifying custom ABIs on a type level in the future2, then it’d be incompatible with this kind of approach. I personally wouldn’t dislike the idea of just blessing this kind of approach though, and to leave any sort of better support for it up to future possibilities.
Speaking of future possibility: I could even imagine a trait like trait CustomAbi<Args, Output> { … }3 that would allow you to somehow define how to use an function of that ABI (on the argument and/or return types that the ABI supports) from Rust. So you can just call foo(x, y) on an extern "custom" fn foo(MyAbi, u32, u32) -> u32 function (or on a function pointer), provided there exists some implementation impl CustomAbi<(u32, u32), u32> for MyAbi.
Alternatively, if we don’t want to discuss any of this at this point, it would in my opinion make a lot of sense to uphold the restriction of only unsafe function for now - in order not to close this kind of future possibility.
Footnotes
-
Imagine one convention is to put a marker type as first argument, another is to put them as last argument, and someone defines some ABIs where basically any Rust type could be handled by passing a pointer - then I can imagine overall-unsound APIs emerging once you create some
extern "custom" fn(AbiConvention1, OtherArg, AbiConvention2)that fits both conventions. ↩ -
Something like
extern "custom(MyAbi)" fn(u32, u32) -> u32perhaps? Orextern "custom"(MyAbi) fn(u32, u32) -> u32, i.e. with somewhat more dedicated syntax to keep the type outside of the string literal? ↩ -
The question of what exactly goes inside of this trait is slightly tricky since there’s currently no way to write something like “
extern "custom" fn(Self, Args…) -> Output” as a type, that “unpacks” a tupleArgsinto function arguments. ↩
There was a problem hiding this comment.
The question of what exactly goes inside of this trait is slightly tricky since there’s currently no way to write something like “
extern "custom" fn(Self, Args…) -> Output” as a type, that “unpacks” a tupleArgsinto function arguments.
There's currently an experimental implementation of fn f(..., #[splat] args: ArgsTuple), tracking issue: rust-lang/rust#153629
I'm skeptical of this. It only gives very partial type safety, because the signature doesn't tell you what the actual ABI is. I could imagine that the illusion of safety might cause more damage than it averts. It also makes it a breaking change to change the signature, which would otherwise be pure documentation (and therefore able to be improved at any time). I think we should have only a single |
This updates section headers to use Markdown 2nd level headings. As part of rust-lang#3883 we switched the template to not use level-1 headings
|
I'll echo that I'm skeptical of the actual benefits of allowing arbitrary signatures. The signature is a lie, if you need additional safety,you can wrap the type. But I'd happily implement allowing it, it does make the docs easier to write. @joshtriplett @scottmcm given Jules' comment, do you see reason to re-evaluate your decision or is that just what T-lang decided and hence what we're going with? Also to clarify
|
|
To clarify, my comment was only about function pointers. I don't object to allowing any signature on the method declaration, though I also think it would be fine to leave it as a future possibility |
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
|
A little uncomfortable that not only an FCP here, but an FCP in the main repo to stabilise both passed while there still seem to be a few active discussions. I don't think we need to block stabilisation on another FCP but I think we should at least address all the feedback on the RFC before merging the feature into stable. |
0d53797 to
edf203f
Compare
edf203f to
6dbabd0
Compare
|
We discussed this again in the T-lang meeting on July 22nd (see notes), deciding to reject argument and return types, but to note that they can be a future extension. Waffle (in DMs) requested that we don't special-case Finally, I've added function pointer checks in rust-lang/rust#159780 and updated the text to reflect that we do enforce the restrictions (must be With that, I believe the RFC text matches the current consensus of T-lang and friends, and can be merged. We can then let the dust settle a bit, and move forward with the stabilization PR in a couple of weeks. I understand that there are some people in the thread here that would like to e.g. see arguments: that door is not closed. It just doesn't seem needed right now. If someone feels they have a compelling use case after stabilization of this minimal version, they can pick up the future extensions. |
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
…ptrs, r=WaffleLapkin check `extern "custom"` function pointers tracking issue: rust-lang#140829 related RFC: rust-lang/rfcs#3980 Best reviewed commit-by-commit. This PR makes 3 changes - extend the ABI checks we already performed on function definitions and trait and foreign declarations to function pointer types. This touches the various `interupt` ABIs and `extern "custom"`. - remove the ability for `extern "custom"` to return `!` - improve the suggestion when `safe` is used in a function pointer type
View all comments
Summary
An
extern "custom" fnis a function with a custom ABI that is unknown to rust. Often these are low-level functions that pass arguments in different registers than any standard calling convention.History
extern "unspecified"for naked functions with arbitrary ABI rust#140566abi_customrust#140829extern "custom"functions rust#140770extern "custom"rust#158504Important
Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.
If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.
Rendered