May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | On Monday, 9 May 2016 at 09:10:19 UTC, Walter Bright wrote:
> Don Clugston pointed out in his DConf 2016 talk that:
>
> float f = 1.30;
> assert(f == 1.30);
>
> will always be false since 1.30 is not representable as a float. However,
>
> float f = 1.30;
> assert(f == cast(float)1.30);
>
> will be true.
>
> So, should the compiler emit a warning for the former case?
(1) Yes, emit a warning for this case.
(2) Generalize it to all variables, like Nordlöw suggested.
(3) Generalize it to all comparisons as well, including < and > .
(4) While we're at it, let's also emit a warning when comparing signed and unsigned types.
(5) Dare I say it... warn against implicit conversions of double to float.
(6) The same applies to "real" as well.
All of these scenarios are capable of producing "incorrect" results, are a source of discrete bugs (often corner cases that we failed to consider and test), and can be hard to detect. It's about time we stopped being stubborn and flagged these things as warnings. Even if they require a special compiler flag and are disabled by default, that's better than nothing.
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | On Monday, May 09, 2016 02:10:19 Walter Bright via Digitalmars-d wrote:
> Don Clugston pointed out in his DConf 2016 talk that:
>
> float f = 1.30;
> assert(f == 1.30);
>
> will always be false since 1.30 is not representable as a float. However,
>
> float f = 1.30;
> assert(f == cast(float)1.30);
>
> will be true.
>
> So, should the compiler emit a warning for the former case?
It does seem like having implicit conversions with floating point numbers is problematic in general, though warning about it or making it illegal could very well be too annoying to be worth it. But at bare minimum, warning about literals not matching the type that they're being compared against when there _is_ a literal that would be of the same type is probably worth warning about - and that could apply to more than just floating point values. But figuring out when implicit conversions are genuinely useful and should be allowed and when they're more trouble than they're worth is surprisingly hard to get right. :(
- Jonathan M Davis
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Xinok | On Monday, 9 May 2016 at 18:37:10 UTC, Xinok wrote: > (1) Yes, emit a warning for this case. > (2) Generalize it to all variables, like Nordlöw suggested. > (3) Generalize it to all comparisons as well, including < and > . > (4) While we're at it, let's also emit a warning when comparing signed and unsigned types. > (5) Dare I say it... warn against implicit conversions of double to float. > (6) The same applies to "real" as well. > > All of these scenarios are capable of producing "incorrect" results, are a source of discrete bugs (often corner cases that we failed to consider and test), and can be hard to detect. It's about time we stopped being stubborn and flagged these things as warnings. Even if they require a special compiler flag and are disabled by default, that's better than nothing. (1) is good, because the code in question is always wrong. (2) is a logical extension, in those cases where constant folding and VRP can prove that the code is always wrong. (3) Makes no sense though; inequalities with mixed floating-point types are perfectly safe. (Well, as safe as any floating-point code can be, anyway.) (4) is already planned; it's just taking *a lot* longer than anticipated to actually implement it: https://issues.dlang.org/show_bug.cgi?id=259 https://github.com/dlang/dmd/pull/1913 https://github.com/dlang/dmd/pull/5229 |
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to tsbockman | On Monday, 9 May 2016 at 18:51:58 UTC, tsbockman wrote:
> On Monday, 9 May 2016 at 18:37:10 UTC, Xinok wrote:
>> ...
>> (3) Generalize it to all comparisons as well, including < and ...
> (3) Makes no sense though; inequalities with mixed floating-point types are perfectly safe. (Well, as safe as any floating-point code can be, anyway.)
> ...
Not necessarily. Reusing 1.3 from the original case, the following assertion passes:
float f = 1.3;
assert(f < 1.3);
And considering we also have <= and >=, we may as well check all of the comparison operators. It's a complex issue because there isn't necessarily right or wrong behavior, only things that *might* be wrong. But if we want to truly detect all possible cases of incorrect behavior, then we have to be exhaustive in our checks.
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Xinok | On Monday, 9 May 2016 at 19:15:20 UTC, Xinok wrote: > It's a complex issue because there isn't necessarily right or wrong behavior, only things that *might* be wrong. But if we want to truly detect all possible cases of incorrect behavior, then we have to be exhaustive in our checks. We absolutely, emphatically, DO NOT want to "detect all possible cases of incorrect behavior". Given the limited information available to the compiler and the intractability of the Halting Problem, the correct algorithm for doing so is this: foreach(line; sourceCode) warning("This line could not be proven correct."); Only warnings with a reasonably high signal-to-noise ratio should be implemented. > Not necessarily. Reusing 1.3 from the original case, the following assertion passes: > > float f = 1.3; > assert(f < 1.3); > > And considering we also have <= and >=, we may as well check all of the comparison operators. Because of the inevitability of rounding errors, FP code generally cannot be allowed to depend upon precise equality comparisons to any value other than zero (even implicit ones as in your example). Warning about this makes sense: float f = 1.3; assert(f == 1.3); Because the `==` makes it clear that the programmer's intent was to test for equality, and that will fail unexpectedly. On the other hand, with `<`, `>`, `<=`, and `>=`, the compiler should generally assume that they are being used correctly, in a way that is tolerant of small rounding errors. Doing otherwise would cause tons of false positives in competently written FP code. Educating programmers who've never studied how to write correct FP code is too complex of a task to implement via compiler warnings. The warnings should be limited to cases that are either obviously wrong, or where the warning is likely to be a net positive even for FP experts. |
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Xinok | On 5/9/2016 11:37 AM, Xinok wrote:
> All of these scenarios are capable of producing "incorrect" results, are a
> source of discrete bugs (often corner cases that we failed to consider and
> test), and can be hard to detect. It's about time we stopped being stubborn and
> flagged these things as warnings. Even if they require a special compiler flag
> and are disabled by default, that's better than nothing.
I've used a B+D language that does as you suggest (Wirth Pascal). It was highly unpleasant to use, as the code became littered with casts. Casts introduce their own set of bugs.
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to tsbockman | On 5/9/2016 11:51 AM, tsbockman wrote:
> On Monday, 9 May 2016 at 18:37:10 UTC, Xinok wrote:
>> (4) While we're at it, let's also emit a warning when comparing signed and
>> unsigned types.
>
> (4) is already planned; it's just taking *a lot* longer than anticipated to
> actually implement it:
> https://issues.dlang.org/show_bug.cgi?id=259
> https://github.com/dlang/dmd/pull/1913
> https://github.com/dlang/dmd/pull/5229
I oppose this change. You'd be better off not having unsigned types at all than this mess, which was Java's choice. But then there are more problems created.
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to tsbockman | On 5/9/2016 12:39 PM, tsbockman wrote:
> Educating programmers who've never studied how to write correct FP code is too
> complex of a task to implement via compiler warnings. The warnings should be
> limited to cases that are either obviously wrong, or where the warning is likely
> to be a net positive even for FP experts.
I've seen a lot of proposals which try to hide the reality of how FP works. The cure is worse than the disease. The same goes for hiding signed/unsigned, and the autodecode mistake of pretending that code units aren't there.
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Steven Schveighoffer | On 5/9/2016 6:46 AM, Steven Schveighoffer wrote:
> I know this is a bit band-aid-ish, but if one is comparing literals to a float,
> why not treat the literal as the type being compared against? In other words,
> imply the 1.3f. This isn't integer-land where promotions do not change the outcome.
Because it's yet another special case, and we know where those lead. For example, what if the 1.30 was the result of CTFE?
|
May 09, 2016 Re: Always false float comparisons | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | Am Mon, 9 May 2016 02:10:19 -0700 schrieb Walter Bright <newshound2@digitalmars.com>: > Don Clugston pointed out in his DConf 2016 talk that: > > float f = 1.30; > assert(f == 1.30); > > will always be false since 1.30 is not representable as a float. However, > > float f = 1.30; > assert(f == cast(float)1.30); > > will be true. > > So, should the compiler emit a warning for the former case? I'd say yes, but exclude the case where it can be statically verified, that the comparison can yield true, because the constant can be losslessly converted to the type of 'f'. By example, don't warn for these: f == 1.0, f == -0.5, f == 3.625, f == 2UL^^60 But do warn for: f == 1.30, f == 2UL^^60+1 As an extension of the existing "comparison is always false/true" check it could read "Comparison is always false: literal 1.30 is not representable as 'float'". There is a whole bunch in this warning category: byte b; if (b == 1000) {} "Comparison is always false: literal 1000 is not representable as 'byte'" -- Marco |
Copyright © 1999-2021 by the D Language Foundation