1289987 - Remove VS2013 workarounds in js/

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: vs2013-js.patch

We no longer support building with VS2013 so these #ifdefs can be removed.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Can the hack in math_sin_uncached (jsmath.cpp) be removed too? The bug in the comment (bug 1076670) mentions VS2013 too.

Comment 2

[Chris Peterson [:cpeterson]]

Some more VS2010 and VS2013 workarounds that can probably be removed: bug 920318, bug 1033146, bug 1143042

Comment 3

[Chris Peterson [:cpeterson]]

Here's a Try run with those other VS2010 and VS2013 workarounds removed:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93e87712038c

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

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

Comment 5

[Chris Peterson [:cpeterson]]

Thanks. I'll post a follow-up patch to remove the VS2013 workarounds and the curly braces.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43490acc4f62
Remove VS2013 workarounds from js/. r=fitzgen

Comment 7

[Chris Peterson [:cpeterson]]

Attachment: part-2-remove-more-workarounds.patch

Part 2: Remove more VS2013 workarounds in js/ (from bug 920318, bug 1033146, and bug 1143042).

Comment 8

[Chris Peterson [:cpeterson]]

Attachment: part-3-remove-curly-braces.patch

Part 3: Remove curly braces workaround for VS2013 ICE in ranged for loops in js/.

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8776063 [details] [diff] [review]
part-2-remove-more-workarounds.patch

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

rs=me

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8776064 [details] [diff] [review]
part-3-remove-curly-braces.patch

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

\o/

Comment 11

[Chris Peterson [:cpeterson]]

Thanks. That was fast!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29315d9869a2
Backed out changeset 2b9b892d36ff for build break

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/974cfc29c1e3
Part 2: Remove more VS2013 workarounds in js/. r=fitzgen

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43490acc4f62
https://hg.mozilla.org/mozilla-central/rev/e2582f36b0e3
https://hg.mozilla.org/mozilla-central/rev/974cfc29c1e3