View Issue Details

IDProjectCategoryView StatusLast Update
0031232FPCRTLpublic2017-02-04 10:04
ReporteravkAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.1.1Product Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0031232: sysutils type helpers does not detect NaN for any float type
Descriptionfunction IsNan always returns False for any float type
TagsNo tags attached.
Fixed in Revision35356
FPCOldBugId
FPCTarget
Attached Files

Activities

avk

2017-01-15 10:34

reporter  

TestNanDetect.lpr (1,592 bytes)

Max Nazhalov

2017-01-15 15:13

reporter   ~0097491

Last edited: 2017-01-15 15:31

View 5 revisions

Hm, not sure it ever supposed to work: IsNan() implemented as, roughly, (Self=Nan), but according to ieee754 NaN is unordered, so it should not be equal even to itself..
Edit: the easiest way to detect NaN is, indeed, either (Self<>Self) or NOT(Self=Self), depending on how strictly ieee754 is followed, and assuming compiler will not optimize out such operation at all.

Dmitriy Pomerantsev

2017-01-15 16:03

reporter   ~0097492

I'll check it tomorrow in Delphi.

Dmitriy Pomerantsev

2017-01-16 17:51

reporter   ~0097524

The results (ExMask/TFPUExceptionMask is replaced by Set8087CW($133F);):

1. 32-bit Windows mode (x87 floating point)

====
Assign NaN to SingleValue;
but Math.IsNan says that SingleValue is NaN

Assign NaN to DoubleValue;
but Math.IsNan says that DoubleValue is NaN

Assign NaN to ExtendedValue;
but Math.IsNan says that ExtendedValue is NaN
====

2. 64-bit Windows mode (sse2 floating point)

====
Assign NaN to SingleValue;
but Math.IsNan says that SingleValue is NaN
and comparision says that SingleValue <> SingleValue

Assign NaN to DoubleValue;
but Math.IsNan says that DoubleValue is NaN
and comparision says that DoubleValue <> DoubleValue

Assign NaN to ExtendedValue;
but Math.IsNan says that ExtendedValue is NaN
and comparision says that ExtendedValue <> ExtendedValue
====

Hmm... May be I have to describe this as bug in Delphi bug tracker? ;-)

Max Nazhalov

2017-01-16 18:13

reporter   ~0097525

I have no latest delphi by hand, but looking into XE -- they just parse bitpattern to see is it NaN or not, so they, probably not fail.
However this approach is very arch-dependent, and should be avoided as much as possible.
But the point is FPC here -- IsNan() is indeed non-functional here for now.

Martok

2017-01-16 20:42

reporter   ~0097528

Testing the byte pattern is the correct way: the definitions of the non-numbers is maximal exponent($7ff..) and mantissa=0 for NaN and mantissa<>0 for infinities (using sign bit for +/-Inf).

This is also how it is impemented in Math.pas. Why not simply forward to IsNan there?

Thaddy de Koning

2017-01-17 08:04

reporter   ~0097536

Last edited: 2017-01-17 08:19

View 4 revisions

@Martok:
That will give the helpers a dependency on math... And therefor systutils. Not a good idea,
Something like including NAN, INFINITY and NEGINFINITY in system, like e.g. Pi, is may be a better idea. Or factoring out the simple type helpers into their own unit.

Martok

2017-01-17 12:06

reporter   ~0097542

Last edited: 2017-01-17 12:09

View 2 revisions

Well. The alternative is duplicating code to a place where this very bug report shows there no test coverage (and users, apparently).

Although arguably it could just be done the other way around, move the implementation to the type helpers and let *Math* just call those. They're all declared inline anyway, so there would be no change in code generated.
€: just checked, Math already depends on SysUtils anyway, so nothing new there either.

avk

2017-01-17 12:29

reporter   ~0097545

Helpers have suitable method "SpecialType".
What about IsNan := SpecialType = fsNan ?

Thaddy de Koning

2017-01-18 08:13

reporter   ~0097565

@Martok
Yes math depends on sysutils but sysutils does not depend on math. The helpers are in sysutils.... Which would make sysutils depend on math. IOW the other way around. That's what my remark is about. And NAN, INFINITY and NEGINFINITY can be trapped without math. E.g. 0.0/0.0 calculation causes NAN without math.. Hence my suggestion to put it in system, just like Pi.

Thaddy de Koning

2017-01-18 08:20

reporter   ~0097566

Last edited: 2017-01-18 08:49

View 5 revisions

What about duplicating the NAN, INFINITY etc functions in the implementation section of syshelpers?
That hides the duplicates but allows the type helpers to use the duplicates to provide the correct results.

I can prepare a patch for that if it is acceptable.
Copying the function bodies would be necessary because comparison operators can not work on the NaN constants, e,g, Nan <> NaN.
[edit]
Note I noticed the consts are already duplicated in the interfaces. See syshelph.inc

Sven Barth

2017-01-20 14:40

manager   ~0097611

Maybe avk's idea of using SpecialType() (also for Is(Negative/Positive)Infinity()) would be easiest...

Thaddy de Koning

2017-01-20 15:44

reporter   ~0097612

Last edited: 2017-01-20 17:33

View 2 revisions

The helpers have also exponent, fraction and mantissa methods.
These work correct. Hence it it easy to fix the NaN and INFINITY by masking these.
I think that may be the easiest way.
http://steve.hollasch.net/cgindex/coding/ieeefloat.html

Michael Van Canneyt

2017-01-29 12:59

administrator   ~0097778

Fixed using system unit T{Double,Single,Extended}rec.SpecialType method.

avk

2017-02-04 10:04

reporter   ~0097959

Thank you!

Issue History

Date Modified Username Field Change
2017-01-15 10:34 avk New Issue
2017-01-15 10:34 avk File Added: TestNanDetect.lpr
2017-01-15 15:13 Max Nazhalov Note Added: 0097491
2017-01-15 15:15 Max Nazhalov Note Edited: 0097491 View Revisions
2017-01-15 15:17 Max Nazhalov Note Edited: 0097491 View Revisions
2017-01-15 15:31 Max Nazhalov Note Edited: 0097491 View Revisions
2017-01-15 15:31 Max Nazhalov Note Edited: 0097491 View Revisions
2017-01-15 16:03 Dmitriy Pomerantsev Note Added: 0097492
2017-01-16 17:51 Dmitriy Pomerantsev Note Added: 0097524
2017-01-16 18:13 Max Nazhalov Note Added: 0097525
2017-01-16 20:42 Martok Note Added: 0097528
2017-01-17 08:04 Thaddy de Koning Note Added: 0097536
2017-01-17 08:13 Thaddy de Koning Note Edited: 0097536 View Revisions
2017-01-17 08:17 Thaddy de Koning Note Edited: 0097536 View Revisions
2017-01-17 08:19 Thaddy de Koning Note Edited: 0097536 View Revisions
2017-01-17 12:06 Martok Note Added: 0097542
2017-01-17 12:09 Martok Note Edited: 0097542 View Revisions
2017-01-17 12:29 avk Note Added: 0097545
2017-01-18 08:13 Thaddy de Koning Note Added: 0097565
2017-01-18 08:20 Thaddy de Koning Note Added: 0097566
2017-01-18 08:22 Thaddy de Koning Note Edited: 0097566 View Revisions
2017-01-18 08:43 Thaddy de Koning Note Edited: 0097566 View Revisions
2017-01-18 08:44 Thaddy de Koning Note Edited: 0097566 View Revisions
2017-01-18 08:49 Thaddy de Koning Note Edited: 0097566 View Revisions
2017-01-20 14:40 Sven Barth Note Added: 0097611
2017-01-20 15:44 Thaddy de Koning Note Added: 0097612
2017-01-20 17:33 Thaddy de Koning Note Edited: 0097612 View Revisions
2017-01-29 12:58 Michael Van Canneyt Assigned To => Michael Van Canneyt
2017-01-29 12:58 Michael Van Canneyt Status new => assigned
2017-01-29 12:59 Michael Van Canneyt Fixed in Revision => 35356
2017-01-29 12:59 Michael Van Canneyt Note Added: 0097778
2017-01-29 12:59 Michael Van Canneyt Status assigned => resolved
2017-01-29 12:59 Michael Van Canneyt Fixed in Version => 3.1.1
2017-01-29 12:59 Michael Van Canneyt Resolution open => fixed
2017-01-29 12:59 Michael Van Canneyt Target Version => 3.2.0
2017-02-04 10:04 avk Note Added: 0097959
2017-02-04 10:04 avk Status resolved => closed