Skip to content
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

Move return so that switch has default case #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eaaltonen
Copy link
Contributor

Commit

  • Move return so that switch has default case

When a project using docopt.cpp is built with -Werror=switch-default
option, there's an error

error: switch missing default case [-Werror=switch-default]

which this fixes.

Signed-off-by: Eero Aaltonen [email protected]

@jaredgrubb
Copy link
Member

This style is intentional.

There is another warning (-Wswitch) that warns if you forget to handle any enum case, but it gets disabled if you add a default (because that's signaling that you intend to cover "everything else not yet mentioned"). However, that's not the case here, as we want to be sure that we always explicitly handle every possible case.

I personally prefer Wswitch-enum which warns even if you have a default, but that warning tends to get very noisy on legacy code-bases so is less used (I don't know this for sure, but just anecdotally).

I personally prefer the style as it is (as it's compatible with both Wswitch and Wswitch-enum). I'll leave this open to hear other opinions though!

When a project using docopt.cpp is built with `-Werror=switch-default`
option, there's an error
```
error: switch missing default case [-Werror=switch-default]
```
which this fixes.

Signed-off-by: Eero Aaltonen <[email protected]>
@eaaltonen eaaltonen force-pushed the fix-swich-missing-default-case branch from 729cbe7 to af11f9a Compare April 11, 2024 10:44
@eaaltonen
Copy link
Contributor Author

I changed the commit so that the warning/error is silenced using a gcc pragma instead. Would you find that acceptable?

@eaaltonen
Copy link
Contributor Author

@jaredgrubb Care to take a look? By suppressing the warning the library could be used also by code that compiles with switch-default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants