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

doc: What is movability in types #17826

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Mar 18, 2022

This adds text explaining what it means for a type to be movable in the context of C, and gives examples.

It does not (yet) declare any type as movable, but contains a TBD note that ipv6_addr_t probably should be as it is already used as that -- given that it's probably already used that way.

I've asked for reviews from people who participated earlier in discussion on this; I don't expect the current state to go through immediately given that with the "used in functions" criterion it diverges a bit from what was previously discussed, but would appreciate your feedback to check the direction.

See-Also: #17817

@github-actions github-actions bot added the Area: doc Area: Documentation label Mar 18, 2022
@chrysn
Copy link
Member Author

chrysn commented Mar 18, 2022

(The IPv6 example may be more relevant than I realized initially, for if the struct were considered opaque and 1-2 functions were different, the implementation might easily flip from having addresses copied around everywhere to storing an 8-bit index into a refcounting NIB that ensures that addresses are cheaper on the stack. We don't do that, and I don't think it pays off with 16 byte addresses, but it could be done that way and then all of a a sudden properties would change. Not on movability mind you but copyability, which is not covered here, but still close enough to be a not-totally-off example.)

chrysn and others added 2 commits March 18, 2022 11:58

Movability
----------

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice explanation, but also slightly verbose and might be missing the "why do we care?".

Something along the lines of:

The C compiler in general doesn't move around data that has been allocated on stack or on the heap by itself, so movability is usually not something that ideomatic C needs to deal with.

The Rust compiler does move data around, if it deems it beneficial for optimization.

As most of RIOT's C APIs are being used by Rust through wrappers, movability of a type needs to be specified in order to no force Rust to assume *may never move* on *all* types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, taken in and modified (because Rust doesn't just move data around either).


This is being documented for consistency within RIOT,
and for the benefit of users who use RIOT
from other languages (such as @rust using-rust "Rust" )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some rust documentation on movability that this could link to?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not like Rust does anything "special" that'd be documented; neither does C strictly forbid moving around values. In particular, the Rust compiler will not willy-nilly move data around, but the ways things are used (RAII without guaranteed return placement, destruction by value) that lead to things being moved.

The Pin<> documentation is obviously related, I've linked it as "verbose and complicated" in the latest version. The proposal for guaranteed copy elision is related as well, but it's not clear enough yet whether it even suffices to link it in here.

@chrysn
Copy link
Member Author

chrysn commented Mar 18, 2022

Thanks for your input, I've pulled in changes and made adjustments.

In particular, I found that the IPv6 example doesn't need more docs (because it is returned by value), and that the lack of a destructor can be a good hint (but no more: not having a destructor / close function / timer clearing function means that the struct is not part of a linked list, but it could still have internal references.)

@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Sep 21, 2022
@chrysn chrysn removed the State: stale State: The issue / PR has no activity for >185 days label Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants