-
Notifications
You must be signed in to change notification settings - Fork 501
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
Spec abi chapter #1545
base: master
Are you sure you want to change the base?
Spec abi chapter #1545
Conversation
src/abi.md
Outdated
* There exists a type `V`, such that `T` is *abi compatible* with `V` an `V` is *abi compatuble* with `U`, | ||
|
||
> [!NOTE] | ||
> These properties ensure that *abi compatibility* is an equivalence relation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding the terms "reflexivity", "symmetry", and "transitivity" to the three bullets above, especially since you use at least the term "transitivity' in the text that follows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a good idea. I wouldn't want to put it on the bullets though, maybe add it to the note.
These properties are respectively called "reflexivity", "symmetry", and "transitivity". They ensure that abi compatibility is an equivalence relation.
src/abi.md
Outdated
|
||
fn main(){ | ||
let f: unsafe fn(*mut ()) = unsafe{core::mem::transmute(foo as unsafe fn(_))}; // Type Erase the function | ||
let mut val = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this example is relying on i32
being the fallback integer type when there is no other alternative being imposed, right?
(E.g., if one had written 5_i8
down below, then that ends up affecting the type inferred for val
, and yields UB overall since now the write will be out-of-bounds, at least according to miri.)
I am wondering whether it would be better, for purposes of this example, to explicitly assign the i32
type via let val: i32 = 0;
and then you avoid discussion of how integer type fallback is handled in this part of the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, yeah. I guess my brain was just on autopilot writing that test.
src/abi.md
Outdated
|
||
|
||
r[abi.compatibility.fn-ptr] | ||
An [`fn`-ptr type] `T` is compatible with an [`fn`-ptr type] `U` if `T` and `U` have *abi compatible* tags. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this supposed to say *abi compatible*
or just compatible
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That one is supposed to say abi compatible, yes.
We need to make mdbook-spec linkify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chorman0773! I really appreciate this!
For now, you'll need to use a fully qualified path like |
That's current what I'm using - the issue is on the |
Ah, I see! Posted #1549 with a fix. |
A concern that I think we should consider is that this seems to duplicate content from the core documentation. I think this is an important question about how we want to handle that. There are a few options:
I think if it is duplicated, it will get out of sync, which I think will contribute to confusion, and cause more work. I lean towards moving it to the reference, but there are some considerations of it being very useful to be in the core docs. |
cc @RalfJung |
I think that where it is relevant, some things should be documented in the standard library even at the cost of duplication and even at the cost of desync. Tersely, and then immediately (ahem) reference the Reference. |
This probably could afford splitting the attributes and argument/return type equivalence into different files? |
8c634cb
to
bcbfdf7
Compare
cc @rust-lang/opsem |
Can you update to also include rust-lang/rust#128784 (assuming that isn't already covered, and should go here and not elsewhere)? |
Does this also need to be updated for rust-lang/rust#110503? |
rust-lang/rust#128784 seems more like something for the
Hmm... I can't tell whether the FCP in rust-lang/rust#130628 provides a just the Layout guarantee for all types or an ABI guarantee, so I'd like for T-lang to clarify that. @rustbot label: +I-lang-nominated Question for T-lang: In rust#130628, T-lang FCP-stabilized a layout guarantee for any |
Co-authored-by: bjorn3 <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
…e new claims about `link_section`.
3092f3c
to
97cd91f
Compare
… abi compatible with their *elision candidate field*s
The new section points to Ralf's UCG chapter temporarily, but the link can be removed and pointed to the id defined by #1654. |
linking external libraries. | ||
|
||
## ABI compatibility | ||
|
||
r[abi.compatibility] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no paragraph after this marker... is this the normal syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's an identifier for the whole section.
> [!NOTE] | ||
> This can include calls to functions defined outside of rust, or built using a different Rust compiler version. | ||
> Additional guarantees will apply in this case for "FFI Safe" types, which match up with the platform C ABI in well-defined ways. | ||
> These are not fully documented here currently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this say that additional requirements apply? The document right now only describes the rules for Rust-to-Rust calls, as far as I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because the note is talking about the fact that "FFI Safe" types have a defined mapping to the platform C abi.
You can technically pass a non-"FFI Safe" type to an FFI call, there's just no guarantee you can match it on the other side.
``` | ||
|
||
r[abi.compatibility.core] | ||
The types [`core::mem::MaybeUninit<T>`], [`core::cell::UnsafeCell<T>`], and [`core::num::NonZero<T>`], are *abi compatible* with `T`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is odd... why do we list these types and not others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These types are magic lang items. MaybeUninit<T>
is a transparent union (which isn't otherwise a thing that can exist), UnsafeCell<T>
doesn't fully qualify as normal repr(transparent)
due to excluding niches (including guaranteed ones that would normally be inherited by repr(transparent)
, and NonZero<T>
has GDE.
Two types, `T` and `U`, are *abi compatible* if both have size 0 and alignment 1. | ||
|
||
r[abi.compatibility.discriminant] | ||
If `T` is an a type listed in [layout.repr.rust.option.elision], and `U` is the type of the *elision candidate field*, then `T` is layout compatible with `U`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The referenced section here doesn't exist yet, does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh this is just a normal markdown link for now, I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…rn type is noted as being part of signature compatibility
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Ralf Jung <[email protected]>
…il after other clauses
This rewrites the
abi
chapter, and adds call compatibility to the chapter.