Wrong warning about loss of precision

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: igor, Assigned: Mardak)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Consider the following section with a debug build of js shell:

js> (1.119).toFixed(2)
(1.119).toFixed(2)
WARNING: A loss of precision for double floating point is detected.
         The result of any operation on doubles can be meaningless.
         A possible cause is missing code to restore FPU state, see
         bug 360282 for details.
1.12
js> 

This warning is incorrect.
(Reporter)

Updated

12 years ago
Summary: Warng warning about loss of precision → Wrong warning about loss of precision

Updated

12 years ago
Blocks: 366848
(Assignee)

Comment 1

11 years ago
Created attachment 279271 [details] [diff] [review]
clear d

Just clear d because we have enough digits already... alternatively, we can duplicate the bump_up code and have that directly goto ret1.
Attachment #279271 - Flags: review?(igor)
(Reporter)

Comment 2

11 years ago
(In reply to comment #1)
> Created an attachment (id=279271) [details]
> clear d
> 
> Just clear d because we have enough digits already... 

Right, the detection of bad floating point requires the loop that covers bump_up label. But shouldn't d be set to 0.0 before another "goto bump_up" as well?
(Assignee)

Comment 3

11 years ago
(In reply to comment #2)
> bump_up label. But shouldn't d be set to 0.0 before another "goto bump_up" as
> well?
You mean the leftright check above for steele&white? I was thinking that, but I wasn't exactly sure how to get the codepath to go to mode 5 to test the change... but just looking at the code, it seems to work.

(agrh. silly bugzilla not giving a CC option when attaching patches)
(Assignee)

Comment 4

11 years ago
Created attachment 279294 [details] [diff] [review]
clear d v2
Attachment #279271 - Attachment is obsolete: true
Attachment #279294 - Flags: review?(igor)
Attachment #279271 - Flags: review?(igor)
(Reporter)

Updated

11 years ago
Attachment #279294 - Flags: review?(igor)
Attachment #279294 - Flags: review+
Attachment #279294 - Flags: approval1.9?
Comment on attachment 279294 [details] [diff] [review]
clear d v2

Use C-style comment, and capitalize the sentence it contains, ending it with a full stop (period character). Both places, r+a=me.

/be
Attachment #279294 - Flags: review+
Attachment #279294 - Flags: approval1.9?
Attachment #279294 - Flags: approval1.9+
(Assignee)

Comment 6

11 years ago
Created attachment 279342 [details] [diff] [review]
clear d v2.1

(In reply to comment #5)
> Use C-style comment, and capitalize the sentence it contains, ending it with a
> full stop (period character).
/* Fixed. */

checkin-needed
Assignee: general → edilee
Attachment #279294 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Reporter)

Comment 7

11 years ago
(In reply to comment #6)
> checkin-needed

I will check it in.
(Reporter)

Updated

11 years ago
Attachment #279342 - Flags: review+
Attachment #279342 - Flags: approval1.9+
(Reporter)

Comment 8

11 years ago
I checked in the patch from comment 6 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1188771562&maxdate=1188771602&who=igor%mir2.org
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 9

11 years ago
How can I capture the warning for a test?

Comment 10

11 years ago
Well, it's stdout, so you could filter that.  Perhaps a better way would be to add a DEBUG-only flag to runtimes indicating whether the warning has happened, set it when the warning is printed, #if DEBUG, and expose some hack to get it in jsshell.  stdout grepping would be nicer than a fairly-bogus member of runtimes or something, tho.
You need to log in before you can comment on or make changes to this bug.