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

Bug in _types.h #195

Open
Vitek1425 opened this issue Apr 21, 2018 · 4 comments · May be fixed by #1510
Open

Bug in _types.h #195

Vitek1425 opened this issue Apr 21, 2018 · 4 comments · May be fixed by #1510

Comments

@Vitek1425
Copy link
Contributor

Vitek1425 commented Apr 21, 2018

constexpr auto type_min = -std::numeric_limits<T>::max();

Is there problem with determination min values here ?
For example -std::numeric_limits::max() != std::numeric_limits::min() also for signed types.
http://en.cppreference.com/w/cpp/types/numeric_limits
Also can replace type_zero to std::numeric_limits::lowest() ?

@Xottab-DUTY
Copy link
Member

6f94e86#diff-1fca930358f9b19260308c7a36188cacL34

I don't know why, but -std::numeric_limits<int>::max() for type_min was used in original and this behaviour was not changed. I don't know why honestly. This issue needs investigation.

@Im-dex, @nitrocaster, any thoughts?

@NeoAnomaly
Copy link
Contributor

NeoAnomaly commented May 21, 2018

That expression is used because the std::numeric_limits::min returns for floating-point types with denormalization the minimum positive normalized value.

For example:

std::numeric_limits<T>::min():
        float: 1.17549e-38 or 0x1p-126
        double: 2.22507e-308 or 0x1p-1022
std::numeric_limits<T>::lowest():
        float: -3.40282e+38 or -0x1.fffffep+127
        double: -1.79769e+308 or -0x1.fffffffffffffp+1023
std::numeric_limits<T>::max():
        float: 3.40282e+38 or 0x1.fffffep+127
        double: 1.79769e+308 or 0x1.fffffffffffffp+1023

In current codebase it would be working correct if std::numeric_limits::min was used

@Xottab-DUTY
Copy link
Member

So, instead of -max() we can use lowest() and leave type_zero as is?

@NeoAnomaly
Copy link
Contributor

Yes, we can... and remove type_zero. IMO, it's absolutely useless )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

Successfully merging a pull request may close this issue.

3 participants