Skip to content

Conversation

jonas-schievink
Copy link
Contributor

Fixes #242

Verified manually that it still prevents linking in multiple versions.

@jonas-schievink jonas-schievink requested a review from a team as a code owner July 12, 2020 00:44
@rust-highfive
Copy link

r? @thejpster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Jul 12, 2020
Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

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

I'm happy with this change to get us lower-cost for the time being, as far as I can tell it preserves the same guarantees while allowing some cases (especially e.g. RTIC) to eliminate the superfluous bool.

However I do note that #239 still has the same issue that both an operating system and an application/driver may need to use cortex-m, but be linked totally separately and only combined at runtime, in which case this "safe" interface is not; perhaps ultimately cortex-m should lose take() and the singleton safety be provided by cortex-m-rt or other runtime.

bors merge

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM

@bors
Copy link
Contributor

bors bot commented Jul 12, 2020

Build succeeded:

@bors bors bot merged commit 437ad12 into rust-embedded:master Jul 12, 2020
@therealprof
Copy link
Contributor

However I do note that #239 still has the same issue that both an operating system and an application/driver may need to use cortex-m, but be linked totally separately and only combined at runtime, in which case this "safe" interface is not; perhaps ultimately cortex-m should lose take() and the singleton safety be provided by cortex-m-rt or other runtime.

I agree.

@jonas-schievink jonas-schievink deleted the zero-means-zero branch July 12, 2020 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove #[no_mangle] from CORE_PERIPHERALS to enable zero-cost peripherals
5 participants