Closed Bug 1303463 Opened 3 years ago Closed 3 years ago

Use safer STRICT_ASSIGN on VisualStudio.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

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: nobody → arai.unmht
Status: NEW → ASSIGNED
Can I have your feedback on this approach?
I'll check performance impact shortly.
Attachment #8792166 - Flags: feedback?(jwalden+bmo)
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.
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)
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 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+
https://hg.mozilla.org/mozilla-central/rev/e407c6185ace
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.
You need to log in before you can comment on or make changes to this bug.