Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

[RFC] rename UserHardFault to HardFault #144

Merged
merged 3 commits into from Nov 14, 2018
Merged

[RFC] rename UserHardFault to HardFault #144

merged 3 commits into from Nov 14, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Oct 27, 2018

so it matches the exception name (#[exception] fn HardFault(..)

Right now the symbol name of all exception handlers match the name of the
function used with the #[exception] attribute except for HardFault, whose
symbol name actually is UserHardFault.

This PR corrects that inconsistency by renaming the UserHardFault symbol to
HardFault, and the HardFault symbol to HardFaultTrampoline.

This change doesn't break compilation or changes functionality but it does soft
break GDB scripts that include the command break UserHardFault (e.g. the GDB
script in cortex-m-quickstart) in the sense that the breakpoint will no longer
work. However the rest of the GDB script will continue to work.

RFC questions: (a) do we want to do this rename? (b) if the answer is yes, do we
want to include this rename in the v0.5.x release, or should we consider it a
breaking change and postpone it until v0.6.0?

@japaric japaric requested a review from a team as a code owner October 27, 2018 12:32
therealprof
therealprof previously approved these changes Oct 27, 2018
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.

I'm all for consistency, I like it.

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.

LGTM.

@thejpster
Copy link
Contributor

I'm happy for this to go out as a v0.5.x.

@adamgreig
Copy link
Member

Likewise happy for it to be in 0.5.x.

korken89
korken89 previously approved these changes Oct 28, 2018
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.

test/compile-fail/hard-fault-twice.rs expected error message needs updating s/User//

@adamgreig adamgreig dismissed stale reviews from korken89 and therealprof via d26d683 November 9, 2018 00:35
@adamgreig
Copy link
Member

@rust-embedded/cortex-m I've fixed the failing build; please re-review.

@japaric
Copy link
Member Author

japaric commented Nov 11, 2018

triage: this has 4 approvals (majority). Changing labels to decision-accepted and S-waiting-on-review.

korken89
korken89 previously approved these changes Nov 13, 2018
@adamgreig
Copy link
Member

bors r=korken89

so it matches the exception name (`#[exception] fn HardFault(..`)
@japaric
Copy link
Member Author

japaric commented Nov 14, 2018

rebased / force pushed to hopefully unstuck bors

therealprof
therealprof previously approved these changes Nov 14, 2018
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

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Nov 14, 2018
144: [RFC] rename UserHardFault to HardFault r=therealprof a=japaric

so it matches the exception name (`#[exception] fn HardFault(..`)

Right now the symbol name of all exception handlers match the name of the
function used with the `#[exception]` attribute *except* for `HardFault`, whose
symbol name actually is `UserHardFault`.

This PR corrects that inconsistency by renaming the `UserHardFault` symbol to
`HardFault`, and the `HardFault` symbol to `HardFaultTrampoline`.

This change doesn't break compilation or changes functionality but it does soft
break GDB scripts that include the command `break UserHardFault` (e.g. the GDB
script in cortex-m-quickstart) in the sense that the breakpoint will no longer
work. However the rest of the GDB script will continue to work.

RFC questions: (a) do we want to do this rename? (b) if the answer is yes, do we
want to include this rename in the v0.5.x release, or should we consider it a
breaking change and postpone it until v0.6.0?

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
@bors
Copy link
Contributor

bors bot commented Nov 14, 2018

Canceled

@japaric
Copy link
Member Author

japaric commented Nov 14, 2018

latest commit should fix the build error

@adamgreig
Copy link
Member

Looks like your rebase got rid of both of my commits, including the one updating the expected error message in the compile-fail test?

@adamgreig
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Nov 14, 2018
144: [RFC] rename UserHardFault to HardFault r=adamgreig a=japaric

so it matches the exception name (`#[exception] fn HardFault(..`)

Right now the symbol name of all exception handlers match the name of the
function used with the `#[exception]` attribute *except* for `HardFault`, whose
symbol name actually is `UserHardFault`.

This PR corrects that inconsistency by renaming the `UserHardFault` symbol to
`HardFault`, and the `HardFault` symbol to `HardFaultTrampoline`.

This change doesn't break compilation or changes functionality but it does soft
break GDB scripts that include the command `break UserHardFault` (e.g. the GDB
script in cortex-m-quickstart) in the sense that the breakpoint will no longer
work. However the rest of the GDB script will continue to work.

RFC questions: (a) do we want to do this rename? (b) if the answer is yes, do we
want to include this rename in the v0.5.x release, or should we consider it a
breaking change and postpone it until v0.6.0?

Co-authored-by: Jorge Aparicio <jorge@japaric.io>
@bors
Copy link
Contributor

bors bot commented Nov 14, 2018

Build succeeded

@bors bors bot merged commit 44ff723 into master Nov 14, 2018
@bors bors bot deleted the hard-fault branch November 14, 2018 01:36
@RandomInsano
Copy link

Hey guys, this seems to have broken the book because of how Semver versioning works:

rust-embedded/book#113

So, the question is should the version of cortext-m-rt have been bumped to 1.0.0 for this as it broke the book? Also, should I re-open that issue?

bors bot added a commit to rust-embedded/book that referenced this pull request Jan 22, 2019
117: Update the references of UserHardFault, which was renamed to HardFault r=korken89 a=sstelfox

This updates the book to match the [renamed symbol](rust-embedded/cortex-m-rt#144) and my [associated PR](rust-embedded/cortex-m-quickstart#64) updating the quickstart repository.

Co-authored-by: Sam Stelfox <sstelfox@bedroomprogrammers.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants