1608089 - Make JSOp an enum class

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Jason Orendorff [:jorendorff]]

For now I'll keep JSOP_GETPROP and so on as aliases for the new names, like JSOp::GetProp, because this touches enough code already that it might be a pain to land.

But in an immediate follow-up bug I'll do the massive search-and-replace.

Comment 1

[Jason Orendorff [:jorendorff]]

Apart from type safety, adding CamelCase names to FOR_EACH_OPCODE will make it easier to keep the Rust frontend up to date now that the integer values of the opcodes have been de-stabilized.

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1608089 - Part 1: Add explicit casts between JSOp and uint8_t. r?jandem.

Later we will turn JSOp into an enum class that requires these casts.

In places where we are overwriting a previously written bytecode,
this casts to jsbytecode rather than uint8_t, following the prevailing
style.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1608089 - Part 2: Make JSOP_LIMIT an int constant rather than a JSOp value. r?jandem.

The change to PendingEdge is OK because this default value is never used; the
field's accessor asserts that *this was created using one of the constructors
that requires an explicit JSOp from the caller.

Depends on D59359

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1608089 - Part 3: Add CamelCase and snake_case identifiers to FOR_EACH_OPCODE. r?jandem.

The snake-case identifiers will be handy in Rust. The plan is to write a script
to import FOR_EACH_OPCODE as a similar Rust macro.

Depends on D59361

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1608089 - Part 4: Convert JSOp to an enum class. r?jandem.

The new opcode names are like JSOp::InitHiddenProp. This patch creates
aliases with all of the old names, like JSOP_INITHIDDENPROP, to keep existing
code working.

Depends on D59362

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0774435321f8
Part 1: Add explicit casts between JSOp and uint8_t. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/ad9edb807ff5
Part 2: Make JSOP_LIMIT an int constant rather than a JSOp value. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/c143928209fb
Part 3: Add CamelCase and snake_case identifiers to FOR_EACH_OPCODE. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/67fdb2aebe67
Part 4: Convert JSOp to an enum class. r=jandem.

Comment 7

[Razvan Maries]

Backed out for build bustages.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=67fdb2aebe674b7c1b939ebbef642c8238d832fe&selectedJob=284720016

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284720016&repo=autoland&lineNumber=26544

Backout: https://hg.mozilla.org/integration/autoland/rev/d2e9558cf7b781521d490318a8c0edd58d327262

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64b7a72f6d92
Part 1: Add explicit casts between JSOp and uint8_t. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/790403ad8b09
Part 2: Make JSOP_LIMIT an int constant rather than a JSOp value. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/936afc4e9720
Part 3: Add CamelCase and snake_case identifiers to FOR_EACH_OPCODE. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/a236919785de
Part 4: Convert JSOp to an enum class. r=jandem.

Comment 9

[Stefan Hindli [:stefan_hindli]]

Backed out 4 changesets (Bug 1608089) for causing build bustages CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=bae27911885940fed6aa65849fe99741c2d0823c&selectedJob=284796181

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284796181&repo=autoland&lineNumber=6497

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2db36e83b324
Backed out 4 changesets for causing build bustages CLOSED TREE

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9e4623c28c6
Part 1: Add explicit casts between JSOp and uint8_t. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/d6e09c1456ae
Part 2: Make JSOP_LIMIT an int constant rather than a JSOp value. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/41c509c1ee6d
Part 3: Add CamelCase and snake_case identifiers to FOR_EACH_OPCODE. r=jandem.
https://hg.mozilla.org/integration/autoland/rev/e8eb26db7f29
Part 4: Convert JSOp to an enum class. r=jandem.

Comment 12

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1608089 - Fix up gdb prettyprinters for JSOp enum, r=jorendorff

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fee2f8a0b01
Fix up gdb prettyprinters for JSOp enum, r=jorendorff

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/c9e4623c28c6
https://hg.mozilla.org/mozilla-central/rev/d6e09c1456ae
https://hg.mozilla.org/mozilla-central/rev/41c509c1ee6d
https://hg.mozilla.org/mozilla-central/rev/e8eb26db7f29

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1fee2f8a0b01