Closed Bug 1493703 Opened 6 years ago Closed 6 years ago

Tech debt: clean up ENABLE_WASM_GC #ifdefs

Categories

(Core :: JavaScript: WebAssembly, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(3 files)

The ENABLE_WASM_GC #ifdefs are applied inconsistently.  They fall into four broad classes: just plain unnecessary (nested within others); safe to remove or adjust without creating more attack surface; controlling the text->binary function; and controlling various switches and hacks that will go away once we have proper tracing.

I'm going to try to land patches for the first two; upload for discussion a patch for the third; and not try to do anything about the fourth at this point, as it just creates turbulence for other changes that will be coming soon.
Attachment #9011489 - Flags: review?(luke)
Attachment #9011490 - Flags: review?(luke)
What's your thinking about this?  There's no danger in applying this, and I guess wasmTextToBinary is not in Firefox in any case, it's just in the shell...
Attachment #9011492 - Flags: feedback?(luke)
Attachment #9011489 - Flags: review?(luke) → review+
Attachment #9011490 - Flags: review?(luke) → review+
Comment on attachment 9011492 [details] [diff] [review]
bug1493703-parser-ifdefs.patch

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

I was trying to think of whether it's useful to have all the GC features in a bucket so we can audit the final set of allowed syntax... but it's not exposed to content so the stakes are low for dangling syntax so removal makes sense.

[Incidentally, WasmTextToBinary.cpp is *built* with Firefox, it's just not installed on any global objects.  It is possible to get wasmTextToBinary via getJSTestingFunctions().wasmTextToBinary if you have access to Components.utils (in chrome JS) or via the worker global scope if automation is enabled.]
Attachment #9011492 - Flags: feedback?(luke) → feedback+
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e57f4697f91f
Remove truly redundant #ifdef ENABLE_WASM_GC. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7fd0abf23ce
Remove more fairly safe #ifdef ENABLE_WASM_GC. r=luke
Keywords: leave-open
Depends on: 1494134
Closing this without further work since further work will happen when we pry apart the #ifdefs for wasm reftypes and wasm gc.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: