1466121 - Rename JSCompartment to JS::Compartment, split vm/JSCompartment.h/cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

At some point we should rename JSCompartment to JS::Compartment (then we'll have JS::Realm, JS::Compartment, JS::Zone).

We should also split vm/JSCompartment.* in vm/Realm.* and vm/Compartment.*

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Rename JSCompartment to JS::Compartment

Mostly very mechanical, except I fixed up some formatting and changed some comments to say realm instead of compartment.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Make Compartment and Zone classes instead of structs

For consistency with JS::Realm, make JS::Compartment and JS::Zone classes instead of structs.

Comment 3

[Jan de Mooij [:jandem]]

I think the next step (after these patches land) is to:

* merge vm/Realm.cpp into vm/JSCompartment.cpp
* hg mv vm/JSCompartment.cpp/h => vm/Compartment.cpp/h
* hg cp vm/Compartment.cpp/h => vm/Realm.cpp/h
  and remove the Compartment code from Realm.cpp/h and remove the Realm code from Compartment.cpp/h

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 8984070 [details] [diff] [review]
Part 2 - Make Compartment and Zone classes instead of structs

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

Nice!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b640dc9b8998
part 1 - Rename JSCompartment to JS::Compartment. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b458961e04
part 2 - Make Compartment and Zone classes instead of structs. r=luke

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/b640dc9b8998
https://hg.mozilla.org/mozilla-central/rev/e3b458961e04

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Merge Realm.cpp into JSCompartment.cpp to simplify later patches

Taking one step back, so we can take two steps forward...

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Part 4 - Rename vm/JSCompartment* to vm/Realm*

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Part 5 - Split Compartment.h from Realm.h

Just a |hg cp| and then I removed code from both files.

The #includes are the hard part because it's not clear which ones we need where. I fixed the obvious ones.

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Part 6 - Split Compartment.cpp from Realm.cpp

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Part 7 - Split Compartment-inl.h from Realm-inl.h

Comment 12

[Jan de Mooij [:jandem]]

I'll probably land each of these parts separately because renaming/adding files can have weird side effects due to unified builds...

Comment 13

[Luke Wagner [:luke]]

Comment on attachment 8984379 [details] [diff] [review]
Part 5 - Split Compartment.h from Realm.h

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

Nice job not losing the history.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db98fad2f4e
part 3 - Merge Realm.cpp into JSCompartment.cpp to simplify later patches. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac87103cdf38
part 4 - Rename vm/JSCompartment* to vm/Realm*. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/af265d75fc00
part 5 - Split Compartment.h from Realm.h. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/45e231683dbf
part 6 - Split Compartment.cpp from Realm.cpp. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/cceb75ca1a1d
part 7 - Split Compartment-inl.h from Realm-inl.h. r=luke

Comment 15

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #12)
> I'll probably land each of these parts separately because renaming/adding
> files can have weird side effects due to unified builds...

I decided not to do this because the stack of patches was green on Try and splitting them up now is probably a bigger risk. Also if there is some weird Talos regression, it's easy enough to bisect on Try because separate revisions.

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/4db98fad2f4e
https://hg.mozilla.org/mozilla-central/rev/ac87103cdf38
https://hg.mozilla.org/mozilla-central/rev/af265d75fc00
https://hg.mozilla.org/mozilla-central/rev/45e231683dbf
https://hg.mozilla.org/mozilla-central/rev/cceb75ca1a1d