Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Changes from all commits
8116b5e
48be95a
fee9c23
b5c0528
15e1563
d10ef05
e10129a
8b7d4df
6d695cd
92c0c94
5c441ea
3f5ca53
b99883a
0d0635b
073ca42
7dd5425
97cd91f
e4c62ac
cbc8349
36f80c3
79a248d
4c219fc
1582d3f
2afe85e
5be0423
ce0915c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
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 normalrepr(transparent)
due to excluding niches (including guaranteed ones that would normally be inherited byrepr(transparent)
, andNonZero<T>
has GDE.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.
Getting rid of the hard dependency on #1654.
If #1654 lands first, then I'll remove the link. If this lands before #1654 I'll make #1645 remove the link.
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.
Can you add a sentence or two here introducing the attribute and briefly explaining what it is for. Generally the template for attributes that we follow is:
The
NAME
attribute ...explain what it is for.Having introductory material is really helpful to give context and immediately understand what the following rules are talking about. It can be helpful to know the purpose up-front, and let the following rules further refine the details.