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

Removing a Double restriction: float support #175

Merged
merged 2 commits into from
Jan 1, 2023
Merged

Conversation

orchestr7
Copy link
Owner

What's done:

  • enabled the decoding of Float type

### What's done:
- enabled the decoding of Float type
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ktlint found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

lineNo: Int,
limits: FloatingPointLimitsEnum,
conversion: (Double) -> T,
): T = if (content in limits.min..limits.max) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's about suggestion to convert to lower type and then back and check that value is the same?

Copy link
Owner Author

@orchestr7 orchestr7 Jan 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nulls , such suggestions valid, however looks a little bit confusing for integer types. But for floating-point types it can cause an error.

Floating-point arithmetics in Kotlin compiler is partially implemented using C++ interop. And in C++ standard it is explicitly told that floating point numbers cannot store all values exactly (not due to limits, but due to precision), so there can potentially be cases when doubleNumber.toFloat().toDouble() != doubleNumber due to loss of bits even when the number is inside Float bounds (-MAX_FLOAT..MAX_FLOAT).

So it is a big question for me if I really should rely on this hack and have potential (but with low probability) case when we corrupt the number and throw an exception (and with a higher complexity algorithm) instead of a simple solution to check of bounds.

As you know, I am not a risky guy, so I think that I would better use low risk option here (at least with floating point numbers). I am sure you would have done the same in this situation... 😄

But If you or @aSemy have very strong objections - I can rework it in the future...

### What's done:
- enabled the decoding of Float type
@orchestr7 orchestr7 merged commit f0f6717 into main Jan 1, 2023
@orchestr7 orchestr7 deleted the feature/float-support branch January 1, 2023 14:26
@orchestr7 orchestr7 linked an issue Jan 5, 2023 that may be closed by this pull request
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.

Followups for integer types
2 participants