Remove VS2013 workarounds in js/

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(3 attachments)

We no longer support building with VS2013 so these #ifdefs can be removed.
Attachment #8775435 - Flags: review?(nfitzgerald)
Can the hack in math_sin_uncached (jsmath.cpp) be removed too? The bug in the comment (bug 1076670) mentions VS2013 too.
Some more VS2010 and VS2013 workarounds that can probably be removed: bug 920318, bug 1033146, bug 1143042
Here's a Try run with those other VS2010 and VS2013 workarounds removed:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93e87712038c
Comment on attachment 8775435 [details] [diff] [review]
vs2013-js.patch

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

\o/

I seem to remember that VS2013 would ICE if you didn't use {} with single statement range based for loops, so we added them in a bunch of places where we wouldn't have otherwise done so. If you feel like hunting them down... Maybe they're gone, as I can't find them anymore.

Also found this, not sure what it is about: http://searchfox.org/mozilla-central/source/js/src/jsapi-tests/testGCHeapPostBarriers.cpp#37
Attachment #8775435 - Flags: review?(nfitzgerald) → review+
Thanks. I'll post a follow-up patch to remove the VS2013 workarounds and the curly braces.
Keywords: leave-open
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43490acc4f62
Remove VS2013 workarounds from js/. r=fitzgen
Part 2: Remove more VS2013 workarounds in js/ (from bug 920318, bug 1033146, and bug 1143042).
Attachment #8776063 - Flags: review?(nfitzgerald)
Part 3: Remove curly braces workaround for VS2013 ICE in ranged for loops in js/.
Attachment #8776064 - Flags: review?(nfitzgerald)
Comment on attachment 8776063 [details] [diff] [review]
part-2-remove-more-workarounds.patch

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

rs=me
Attachment #8776063 - Flags: review?(nfitzgerald) → review+
Comment on attachment 8776064 [details] [diff] [review]
part-3-remove-curly-braces.patch

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

\o/
Attachment #8776064 - Flags: review?(nfitzgerald) → review+
Thanks. That was fast!
Keywords: leave-open
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9b892d36ff
Part 2: Remove more VS2013 workarounds in js/. r=fitzgen
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2582f36b0e3
Part 3: Remove curly braces workaround for VS2013 ICE in ranged for loops in js/. r=fitzgen
Backout by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29315d9869a2
Backed out changeset 2b9b892d36ff for build break
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/974cfc29c1e3
Part 2: Remove more VS2013 workarounds in js/. r=fitzgen
You need to log in before you can comment on or make changes to this bug.