Use safer STRICT_ASSIGN on VisualStudio.

RESOLVED FIXED in Firefox 52

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox51 wontfix, firefox52 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
As discovered in bug 1303085, VisualStudio doesn't do STRICT_ASSIGN as expected on float on some case.
changing it to use safer version solves the issue.
(Assignee)

Updated

2 years ago
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
Created attachment 8792166 [details] [diff] [review]
Use safer STRICT_ASSIGN on VisualStudio.

Can I have your feedback on this approach?
I'll check performance impact shortly.
Attachment #8792166 - Flags: feedback?(jwalden+bmo)
(Assignee)

Comment 4

2 years ago
I don't see any performance difference between them, with the following code:

function f() {
  var y = 0;
  for (var x = -100; x <= 100; x += 0.0001) {
    y += Math.exp(x) * 0.00000001;
  }
  return y;
}

result on my windows 10 laptop (32bit)
  before: 617.4ms
  after:  614.1ms

we could go with it.
(Assignee)

Comment 5

2 years ago
Comment on attachment 8792166 [details] [diff] [review]
Use safer STRICT_ASSIGN on VisualStudio.

so, changing to safer STRICT_ASSIGN doesn't introduce any performance issue,
and it solves the issue in bug 1303085.
Attachment #8792166 - Flags: feedback?(jwalden+bmo) → review?(jwalden+bmo)
(Assignee)

Comment 6

2 years ago
so far, I confirmed the issue about correctness only with float type, and we don't have STRICT_ASSIGN with float in tree.
so this issue won't affect existing functions.

Comment 7

2 years ago
Comment on attachment 8792166 [details] [diff] [review]
Use safer STRICT_ASSIGN on VisualStudio.

Review of attachment 8792166 [details] [diff] [review]:
-----------------------------------------------------------------

This is, um, super rubberstampy.
Attachment #8792166 - Flags: review?(jwalden+bmo) → review+

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e407c6185ace
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.
status-firefox51: affected → wontfix
You need to log in before you can comment on or make changes to this bug.