Skip to content
This repository was archived by the owner on Mar 7, 2026. It is now read-only.

Fix: warning pedantic range expr#1603

Merged
dragonmux merged 4 commits intoblackmagic-debug:mainfrom
perigoso:fix/warning-pedantic-range-expr
Aug 19, 2023
Merged

Fix: warning pedantic range expr#1603
dragonmux merged 4 commits intoblackmagic-debug:mainfrom
perigoso:fix/warning-pedantic-range-expr

Conversation

@perigoso
Copy link
Copy Markdown
Contributor

Detailed description

This one was missed from the initial batch

Fix (some) warnings for -Wpedantic

See #1590 for context

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

This is looking good. There's one item that we have queries on that this touches but with that resolved, think this is good for merge.

Might we suggest, given this touches the Cortex-A and Cortex-M core support, that you give feature/arm-cortex-refactoring a look as that changes a fair whack to do with the Cortex code, as does feature/cortex-r-support which is built on top of the previously mentioned branch?

@dragonmux dragonmux added this to the v1.10 milestone Aug 17, 2023
@dragonmux dragonmux added the Enhancement General project improvement label Aug 17, 2023
@dragonmux dragonmux mentioned this pull request Aug 14, 2023
43 tasks
@perigoso perigoso force-pushed the fix/warning-pedantic-range-expr branch 2 times, most recently from a0890a6 to 58a3a2c Compare August 18, 2023 13:26
Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

Only have one comment that, once answered, leaves us happy to merge this. Great work! We look forward to finding a resolution to the enum problem too in a follow-up PR

range expressions in switch statements are non-standard
0x3 type is not defined, but the table did not account for its offset, meaning
every type > 0x3 would should show a wrong, offset by 1 string, and type 8 would be
out of bounds
@perigoso perigoso requested a review from dragonmux August 18, 2023 23:29
@perigoso perigoso force-pushed the fix/warning-pedantic-range-expr branch from 3aa045a to 7136557 Compare August 18, 2023 23:30
Copy link
Copy Markdown
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

LGTM, we'll merge this once we've given it a test against some local parts to make sure things seem sane.

@dragonmux
Copy link
Copy Markdown
Member

Looks good, seems to work well. Merging!

@dragonmux dragonmux merged commit 2bd006b into blackmagic-debug:main Aug 19, 2023
@perigoso perigoso deleted the fix/warning-pedantic-range-expr branch August 19, 2023 21:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Enhancement General project improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants