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

noexcept all the things #74

Merged
merged 1 commit into from
Dec 30, 2020

Conversation

lgritz
Copy link
Contributor

@lgritz lgritz commented Dec 18, 2020

This patch marks as noexcept absolutely everything that could be under
C++11 rules. Excluded are, obviously, anything that intentionally throws,
and those are very few after Owen's recent Imath overhaul and my changes
to eliminate the weird integer versions of normalize et al.

This patch also makes most of numeric_limits constexpr, per
C++11 rules. But a few are not constexpr yet because parts of half
still need to be fully constexpr-plumbed, and there are some tricky
parts (and maybe not 100% possible in C++11). I may revisit this in a
later patch.

Using noexcept is a promise that the caller to your function won't need
to handle exceptions. If your function calls something that turns out to
throw, it's on you to catch it, or else the application will crash with
an uncaught exception.

It's important that this get a thorough review. I really need a few
keen eyes to make sure that I haven't inadvertently marked anything
noexcept if it calls things (including outside Imath itself) that
might throw exceptions. (I mean, we can fix those bugs later, but I'd
prefer to find them now rather than in the heat of production.)

Closes #64

Signed-off-by: Larry Gritz lg@larrygritz.com

This patch marks as noexcept absolutely everything that could be under
C++11 rules. Excluded are, obviously, anything that intentionally throws,
and those are very few after Owen's recent Imath overhaul and my changes
to eliminate the weird integer versions of normalize et al.

This patch also makes most of numeric_limits<half> constexpr, per
C++11 rules.  But a few are not constexpr yet because parts of half
still need to be fully constexpr-plumbed, and there are some tricky
parts (and maybe not 100% possible in C++11). I may revisit this in a
later patch.

Using `noexcept` is a promise that the caller to your function won't need
to handle exceptions. If your function calls something that turns out to
throw, it's on you to catch it, or else the application will crash with
an uncaught exception.

It's important that this get a thorough review. I really need a few
keen eyes to make sure that I haven't inadvertently marked anything
noexcept if it calls things (including outside Imath itself) that
might throw exceptions. (I mean, we can fix those bugs later, but I'd
prefer to find them now rather than in the heat of production.)

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Copy link
Contributor

@meshula meshula 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 one of those diffs that seems to need one of the specialized Japanese "thank you" variants, like gokorusama deshita, for use on completion of a mundane but necessary task with a significant amount of work expended but no particular glamour... Which is to say, thanks for taking on the task!

I tried to find potential issues, such as nan's in calculations on frustums with a zero fov, and I didn't spot any problems.

@lgritz
Copy link
Contributor Author

lgritz commented Dec 23, 2020

I'll give this through the weekend for anybody else to inspect and/or complain, will merge next week if I haven't seen any objections.

@lgritz lgritz merged commit 99659db into AcademySoftwareFoundation:master Dec 30, 2020
@lgritz lgritz deleted the lg-noexcept branch December 30, 2020 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imath is lacking noexcept declarators
3 participants