1439004 - build bustage in js/src/wasm/WasmBinaryToText.cpp when Gecko 60 merges to Beta on 2018-03-01

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

When Gecko 60 merges to beta, the builds will fail.

Try push with build failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc1afe3d70a131e2022bc470b0b78a695fc95cee&selectedJob=162731903
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=162731903&repo=try

js/src/wasm/WasmBinaryToText.cpp:754:53: error: 'AstExtraConversionOperator' has not been declared

The code landed in bug 1435369: https://hg.mozilla.org/mozilla-central/rev/25900f3b9936#l30.12

Comment 1

[Jakob Stoklund Olesen [:jolesen]]

I am also seeing this build error when I remove DEFINES['ENABLE_WASM_SATURATING_TRUNC_OPS'] = True from moz.build.

/Users/jolesen/gecko-dev/js/src/wasm/WasmBinaryToText.cpp:754:53: error: unknown type name 'AstExtraConversionOperator'; did you mean 'AstConversionOperator'?
RenderExtraConversionOperator(WasmRenderContext& c, AstExtraConversionOperator& conv)
                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
                                                    AstConversionOperator

/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:302:17: warning: enumeration value 'ExtraConversionOpcode' not handled in switch [-Wswitch]
        switch (kind_) {
                ^
/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:302:17: note: add missing switch cases
        switch (kind_) {
                ^
/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:4318:18: warning: enumeration value 'ExtraConversionOperator' not handled in switch [-Wswitch]
    switch (expr.kind()) {
                 ^
/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:4318:18: note: add missing switch cases
    switch (expr.kind()) {
                 ^
/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:4930:18: warning: enumeration value 'ExtraConversionOperator' not handled in switch [-Wswitch]
    switch (expr.kind()) {
                 ^
/Users/jolesen/gecko-dev/js/src/wasm/WasmTextToBinary.cpp:4930:18: note: add missing switch cases
    switch (expr.kind()) {
                 ^

Comment 2

[Dan Gohman [:sunfish]]

Attachment: bug1439004.patch

Fixed in the attached patch. I had to pick between removing the `#ifdef`s around AstExtraConversionOperator and adding `#ifdef`s around code that uses it; I opted to remove `#ifdef`s in the patch here, because it keeps the code simpler, however I'm happy to do it either way.

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 8952492 [details] [diff] [review]
bug1439004.patch

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

Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdf244d72f44
Fix build breakage when ENABLE_WASM_SATURATING_TRUNC_OPS is not defined. r=luke

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/cdf244d72f44

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The patch didn't catch everything and beta would still be busted:

Simulation push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5b71f018f2ab5aac5cfae2293bd9b56a6b28d6c
Log: https://treeherder.mozilla.org/logviewer.html#?job_id=163512672&repo=try

js/src/wasm/WasmTextToBinary.cpp:301:16: error: enumeration value 'ExtraConversionOpcode' not handled in switch [-Werror=switch]

Problematic code: https://dxr.mozilla.org/mozilla-central/rev/861067332bac96a44bbf41ef366f58a30476057b/js/src/wasm/WasmTextToBinary.cpp#316-318

Comment 7

[Dan Gohman [:sunfish]]

Attachment: more-ifdefs.patch

My test build didn't have --enable-warnings-as-errors, and I missed the warning. Attached is a follow-up patch. I'm now testing it on try here, with a modification to undefined ENABLE_WASM_SATURATING_TRUNC_OPS:

https://hg.mozilla.org/try/rev/ca79d4fef803a0e7ea479437663eb086f0b2ab7d

Comment 8

[Dan Gohman [:sunfish]]

Comment on attachment 8952839 [details] [diff] [review]
more-ifdefs.patch

Here's the try build URL:

https://treeherder.mozilla.org/#/jobs?repo=try&
revision=655482e95c572b9985740240581af97a3fcd70f1

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4f510195124
Fix more build breakage when ENABLE_WASM_SATURATING_TRUNC_OPS is not defined. r=luke

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a4f510195124