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 numeric_limits<half> specializations into half.h #241

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

cary-ilm
Copy link
Member

Previously, these specializations have been in the header halfLimit.h,
but without this header, the numeric_limits<> template provides
default specializations that return 0, so, for example,
std::numeric_limits<half>::min() would silently return 0 in code that
inadvertently omitted the #include <halfLimits.h>.

The numeric_limits specializations should be considered an inseparable
part of the class definition.

This change moves the specializations directly into half.h and
deprecates the halfLimits.h header altogether. For backwards
compatibility, it includes half.h with a warning.

An alternative would be to keep halfLimits.h and include it in half.h,
but then each header would include the other circularly, which is
awkward.

Should address #240.

Signed-off-by: Cary Phillips cary@ilm.com

Previously, these specializations have been in the header halfLimit.h,
but without this header, the numeric_limits<> template provides
default specializations that return 0, so, for example,
std::numeric_limits<half>::min() would silently return 0 in code that
inadvertently omitted the #include <halfLimits.h>.

The numeric_limits specializations should be considered an inseparable
part of the class definition.

This change moves the specializations directly into half.h and
deprecates the halfLimits.h header altogether. For backwards
compatibility, it includes half.h with a warning.

An alternative would be to keep halfLimits.h and include it in half.h,
but then each header would include the other circularly, which is
awkward.

Signed-off-by: Cary Phillips <cary@ilm.com>
@lgritz
Copy link
Contributor

lgritz commented Mar 16, 2022

As far as I'm concerned, the closer we get to the entire universe of half being in a single self-contained header file, the better.

@cary-ilm cary-ilm merged commit 71e2215 into AcademySoftwareFoundation:main Mar 16, 2022
cary-ilm added a commit to cary-ilm/Imath that referenced this pull request Mar 22, 2022
…eFoundation#241)

Previously, these specializations have been in the header halfLimit.h,
but without this header, the numeric_limits<> template provides
default specializations that return 0, so, for example,
std::numeric_limits<half>::min() would silently return 0 in code that
inadvertently omitted the #include <halfLimits.h>.

The numeric_limits specializations should be considered an inseparable
part of the class definition.

This change moves the specializations directly into half.h and
deprecates the halfLimits.h header altogether. For backwards
compatibility, it includes half.h with a warning.

An alternative would be to keep halfLimits.h and include it in half.h,
but then each header would include the other circularly, which is
awkward.

Signed-off-by: Cary Phillips <cary@ilm.com>
cary-ilm added a commit that referenced this pull request Mar 25, 2022
Previously, these specializations have been in the header halfLimit.h,
but without this header, the numeric_limits<> template provides
default specializations that return 0, so, for example,
std::numeric_limits<half>::min() would silently return 0 in code that
inadvertently omitted the #include <halfLimits.h>.

The numeric_limits specializations should be considered an inseparable
part of the class definition.

This change moves the specializations directly into half.h and
deprecates the halfLimits.h header altogether. For backwards
compatibility, it includes half.h with a warning.

An alternative would be to keep halfLimits.h and include it in half.h,
but then each header would include the other circularly, which is
awkward.

Signed-off-by: Cary Phillips <cary@ilm.com>
@alvinhochun
Copy link

I slightly disagree with the deprecation warning. Some projects may see the warning and think "oh, we will just remove the include / replace the include with half.h", without realizing that this may cause regressions when someone compiles the project with an older version of OpenEXR or Imath. If you must have the deprecation warning, it should at least mention the risk of silently causing problems when built with older versions.

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

Successfully merging this pull request may close these issues.

None yet

4 participants