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

Remove useless noexcept(false) from narrow #1152

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

Mq-b
Copy link
Contributor

@Mq-b Mq-b commented May 15, 2024

I thought this was some kind of code specification at first, but I searched the entire repository and only saw this one use of noexcept(false). I don't think that's necessary here, narrow is not a destructor.

If no exception specification is explicitly provided, the exception specification is considered to be one that would be used by the implicitly-declared destructor (see below). In most cases, this is noexcept(true). Thus a throwing destructor must be explicitly declared noexcept(false).

I looked at other gsl libraries and didn't see 'noexcept(false)' used either.

https://github.com/gsl-lite/gsl-lite/blob/bd9eb162d42d8ae6a7e86902ca7060247d71ac41/include/gsl/gsl-lite.hpp#L2532

@Mq-b Mq-b changed the title Remove useless 'noexcept(false)' from 'narrow' Remove useless noexcept(false) from narrow May 15, 2024
@frederick-vs-ja
Copy link

frederick-vs-ja commented May 16, 2024

No change requested - Instead of implicit or explicit noexcept(false), would it be better to use conditional noexcept here?

@d-winsor
Copy link
Collaborator

d-winsor commented Jul 17, 2024

@Mq-b There appears to be problems with the checks running on older PRs. Can you please push an empty commit to retrigger tests.

@carsonRadtke carsonRadtke merged commit 32511b8 into microsoft:main Oct 14, 2024
95 checks passed
@carsonRadtke
Copy link
Collaborator

@Mq-b Thanks for the contribution!

@Mq-b
Copy link
Contributor Author

Mq-b commented Oct 14, 2024

@carsonRadtke Thank you for merging the PR! I'm glad I could contribute. Looking forward to more opportunities to collaborate and improve the project!

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.

5 participants