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

cgen: use standard checks for float comparisons #5203

Merged
merged 5 commits into from
Jun 4, 2020

Conversation

UweKrueger
Copy link
Member

@UweKrueger UweKrueger commented Jun 3, 2020

Since there was only positive feedback to #5180 I've created this PR that switches to standard float checks. It also removes code that is not needed any more.

Sorry to whoever has implemented this! The work was not in vain since a very similar approach is will probably be needed for other comparison cases.

@UweKrueger UweKrueger marked this pull request as ready for review June 3, 2020 22:04
@medvednikov
Copy link
Member

I think we should leave f32.eq() or f32.eq_epsilon or math.compare_floats() or something

@spytheman
Copy link
Member

Sigh... This fairly known problem, will then continue to surprise novices in programming, even in V...

@UweKrueger
Copy link
Member Author

@medvednikov I've added f64.eq_epsilon() and f32.eq_epsilon(). Since these are not used by default, I've spent some effort to make them work in the whole normalized range of these types (0,±FLT_MIN..±FLT_MAX, denormalized numbers < FLT_MIN might be problematic).
@spytheman surprises can be fun ;-) Honestly, I think the only way to really avoid surprises is implementing arbitrary precision symbolic math with the ability to proof the equality of two differently structured expressions. But this goes in the direction computer algebra system or formal verification and lies definitely beyond the scope of the v core language.
Once upon a time there were computer systems that used internal decimal representation of numbers (and pocket calculators do so even today). This makes at least what you see is what you get and avoids some surprises. gcc supports IEEE754 decimal floats, however due to lack of hardware support the performance is limited.

@medvednikov medvednikov merged commit cf9498e into vlang:master Jun 4, 2020
@medvednikov
Copy link
Member

@UweKrueger thanks!

By the way I just found out that enum = 1 works, enum = Enum(1) must be required.

@UweKrueger UweKrueger deleted the default_flt_checks branch June 4, 2020 19:12
@dumblob
Copy link
Contributor

dumblob commented Aug 18, 2020

I finally posted my "opus magnum" 😉 to strongly encourage you to rething float comparisons once again - see #5180 (comment) .

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.

None yet

4 participants