Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Oct 24, 2017

Emit a warning when an implicit conversion from a FP type to another may lead to lose the accuracy of the RHS. Requires a phobos and a druntime change to pass the tests.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close Bugzilla Description
17933 Warn in case of implicit truncating float conversion

@andralex
Copy link
Member

@ibuclaw
Copy link
Member

ibuclaw commented Oct 24, 2017

Seems like a reasonable addition to me.

@andralex
Copy link
Member

If this is deemed a worthy addition, it should be a deprecation not a warning. We should eliminate all warnings.

Historically @WalterBright and I have considered conversions from double to float acceptable, so this change (which will break code) needs to be argued properly.

@don-clugston-sociomantic
Copy link
Contributor

Suppose somebody gets this warning. How do they suppress it? You might think you can do:
f = cast(float)d; f += 0.1f;
But this is still actually performing a truncation!
The problem is that 0.1f isn't actually a float literal. It's a double pretending to be a float.

Until this static assert passes, this PR should be rejected.
static assert(0.1f != 0.1);
It really should fail, 0.1 stored in a float and 0.1 stored in a double are two different values. But in D this is broken.
But even then, you'd also need to change the meaning of cast(float) so that type painting is distinguished from truncation.

The underlying problem is that in practice, D is defined to use x87 floating-point semantics, where float and double and real are (almost) the same internally. These semantics work extremely poorly on any other CPU. But unless we tighten our semantics, we can't accept PRs like this one.

@ghost
Copy link
Author

ghost commented Oct 28, 2017

It's too late and the required changes make the code ugly. The related phobos PR still misses about 50 unfixed warnings.

This pull request was closed.
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.

4 participants