10000 make TakeCell::empty() const, and make nrf52840 GPIO pin initialization a const fn by folkertdev · Pull Request #3937 · tock/tock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

make TakeCell::empty() const, and make nrf52840 GPIO pin initialization a const fn #3937

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

Merged
merged 2 commits into from
Apr 7, 2024

Conversation

folkertdev
Copy link
Contributor

Pull Request Overview

This pull request makes the TakeCell::new function const. This requires an associated constant: moving the constant into fn new does not work, not using an explicit constant also does not work.

That function being const can then be used to initialize the gpio pins for the ntrf52840 in a const fn. It can't happen in an actual const because the final type contains a (non-static) lifetime.

There are many more functions that can now become a const fn: I can add some to this PR but those are scattered all around the codebase.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

because of lifetimes the initialization cannot (yet) happen in a const
@github-actions github-actions bot added nrf Change pertains to the nRF5x family of MCUs. tock-libraries This affects libraries supported by the Tock project WG-Network In the purview of the Network working group. labels Mar 26, 2024
@bradjc
Copy link
Contributor
bradjc commented Mar 26, 2024

This effectively reverts 4a465e6, and implements #2214.

I assume this is due to improvements in the (stable) Rust compiler. I think this is a change we want, but wanted to link the history.

@folkertdev
Copy link
Contributor Author
folkertdev commented Mar 26, 2024

This trick has actually worked for a long time (apparently, since 1.32 https://godbolt.org/z/5bWhYf5fP), but only because the const is defined as an associated constant. The mutable reference in a const error is triggered when you put the const elsewhere (e.g. in the body of empty)

brghena
brghena previously approved these changes Mar 27, 2024
@brghena
Copy link
Contributor
brghena commented Mar 27, 2024

Bringing to the attention of @lschuermann

Copy link
Contributor
@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

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

One comment, otherwise looks good

@bradjc bradjc changed the title make nrf52840 GPIO pin initialization a const fn make TakeCell::empty() const, and make nrf52840 GPIO pin initialization a const fn Mar 29, 2024
Copy link
Member
@lschuermann lschuermann left a comment

Choose a reason for hiding this comment

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

Sorry for my late response; this looks good to me. I was pretty sure that this would still allow creation of trait objects due to this associated const not being part of the trait, but wanted to double check. This still works, so we won't run into any issues in the future here!

@lschuermann lschuermann added this pull request to the merge queue Apr 7, 2024
Merged via the queue into tock:master with commit 868d0b1 Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nrf Change pertains to the nRF5x family of MCUs. tock-libraries This affects libraries supported by the Tock project WG-Network In the purview of the Network working group.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0