Prepare for v0.7.5 #441
Merged
Prepare for v0.7.5 #441
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
380: Improve singleton!() macro r=adamgreig a=Rahix This PR addresses two shortcomings of the `cortex_m::singleton!()` macro, which I raised in #364. For review, I think it is best to look at the two commits implementing these changes individually. I think this changeset should also be backported to `0.7.x` where it applies cleanly and which is also the place where I tested it. The static is always initialized to a "zero" value with `Option::None` which means it should end up in `.bss`. However, if the enclosed type has a niche, `Option::None` can become a non-zero bitpattern which moves the whole singleton from `.bss` to `.data`. This is especially problematic when storing large buffers in the `singleton!()` as this starts eating lots of flash space unnecessarily. To prevent this, I switched to using an explicit boolean flag instead. This is not quite as nice but at least there is no chance for the `singleton!()` to end up in `.data`... For reference and as an example, the faulty behavior can be triggered with ```rust cortex_m::singleton!(: Option<u32> = None) ``` (the inner option has a non-zero niche which the outer option will occupy) Due to the static always being named `VAR` right now, all `singleton!()` instances end up having non-descriptive symbol names like `__cortex_m_rt_main::{{closure}}::VAR` which makes them hard to tell apart in a debugger or when looking at an objdump. I added the ability to set an explicit name which end up becoming part of the symbol name. This does not affect Rust code at all - the new name is not visible anywhere. It works like this: ```rust let value = singleton!(FOO_BUFFER: [u8; 1024] = [0u8; 1024]); ``` Of course the old syntax also still works and keeps the old behavior of calling the static `VAR`. Fixes #364. Co-authored-by: Rahix <[email protected]>
newAM
approved these changes
May 18, 2022
|
I've tested this and confirmed that code using one of the asm functions, when |
|
So the only thing to be aware of is #442 because if we get a PR for that, we'll almost certainly get a request to push it out in a new release. |
|
Since it looks like #442 relates to cortex-m-semihosting instead, I don't think it's a blocker for this. |
|
bors r+ |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently with cortex-m 0.7.4 it's not possible for stable Rust users to enable the inline-asm feature, even though their compiler might support it, because of the
![cfg_attr(feature = "inline-asm", feature(asm))]line. I propose a new 0.7.5 release that removes this line, which means users on stable Rust >=1.59 could enable the previously nightly-only feature to get stable inline asm.I wouldn't enable the feature by default, because that would be a significant MSRV bump, but at least this way more users could enable it before we release 0.8 with the revamped and inline-only asm.
I've also backported the bugfix from #380 which I don't believe is a breaking change.
I haven't had a chance to test this yet so it would be great if someone could try it out and just make sure the inline-asm feature does work before merging.
Any thoughts on anything else worth backporting from 0.8?