1723155 - Fix multiple private methods bugs

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[André Bargull [:anba]]

Fix the following private method bugs:

• Disallow installing private brand multiple times.
• Make private static methods non-writable.
• Install private static NoPrivateGetter on the constructor instead of the class prototype object.
• Ensure private non-static methods always use the correct home-object.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 1: Disallow installing the private brand more than once. r=mgaudet!

Similar to private fields, we must throw an error on attempts to install the
private brand multiple times.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 2: EmitterScope::lookupPrivate() is infallible. r=mgaudet!

Drive-by change:
This method is no longer fallible, so we can change the return type to void.

Depends on D121735

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 3: Split private names from computed names handling. r=mgaudet!

Split private names in preparation for the next parts.

Depends on D121736

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 4: Fix some stack comments. r=mgaudet!

CTOR is always present when emitting classes.

Depends on D121737

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 5: Private static methods aren't writable. r=mgaudet!

https://tc39.es/ecma262/#sec-privateset, step 4 requires to throw a TypeError.

Depends on D121738

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 6: Install static private getter on the constructor. r=mgaudet!

We could optimise this case by not popping CTOR from the stack in
emitPrivateStaticMethod, but that requires more changes in PropertyEmitter.

Depends on D121739

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 7: Reset internal state when emitting private non-static methods. r=mgaudet!

This change is necessary to ensure the correct home-object can be found.

Depends on D121740

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 8: Add NewPrivateName opcode. r=mgaudet!

Fix this TODO by adding a new opcode JSOp::NewPrivateName.

Depends on D121741

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 9: Remove Maybe around source positions. r=mgaudet!

Source positions used mozilla::Maybe<uint32_t> for BinAST, but with BinAST
removed, we can now change this back to use uint32_t.

Depends on D121742

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 10: Cleanup BytecodeCompilation header. r=mgaudet!

While removing unnecessary mozilla::Maybe in the last part, I've noticed that
this header has many unused or unnecessary includes.

Depends on D121743

Comment 12

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 11: Always assert the property key is a private name in CheckPrivateFieldOperation. r=mgaudet!

And remove some commented out debugging code.

Depends on D121745

Comment 13

[André Bargull [:anba]]

Attachment: Bug 1723155 - Part 12: Remove always false condition from emitBrandCheck(). r=mgaudet!

If isFieldInit() returns true, the first if-statement is taken, so it must
be false in the else block.

Depends on D121746

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d2d24449e42
Part 1: Disallow installing the private brand more than once. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4036a68b398a
Part 2: EmitterScope::lookupPrivate() is infallible. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d8c484ec3902
Part 3: Split private names from computed names handling. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/6f8f34ed2aa5
Part 4: Fix some stack comments. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/9b7e03966d7f
Part 5: Private static methods aren't writable. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4704eb55b46a
Part 6: Install static private getter on the constructor. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/f49a1b1fbd9b
Part 7: Reset internal state when emitting private non-static methods. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/5e6d1848ce62
Part 8: Add NewPrivateName opcode. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/32bb1dfaf41b
Part 9: Remove Maybe around source positions. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e3f2042fa018
Part 10: Cleanup BytecodeCompilation header. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/af96ac52d947
Part 11: Always assert the property key is a private name in CheckPrivateFieldOperation. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/0ab8644c9e71
Part 12: Remove always false condition from emitBrandCheck(). r=mgaudet

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0d2d24449e42
https://hg.mozilla.org/mozilla-central/rev/4036a68b398a
https://hg.mozilla.org/mozilla-central/rev/d8c484ec3902
https://hg.mozilla.org/mozilla-central/rev/6f8f34ed2aa5
https://hg.mozilla.org/mozilla-central/rev/9b7e03966d7f
https://hg.mozilla.org/mozilla-central/rev/4704eb55b46a
https://hg.mozilla.org/mozilla-central/rev/f49a1b1fbd9b
https://hg.mozilla.org/mozilla-central/rev/5e6d1848ce62
https://hg.mozilla.org/mozilla-central/rev/32bb1dfaf41b
https://hg.mozilla.org/mozilla-central/rev/e3f2042fa018
https://hg.mozilla.org/mozilla-central/rev/af96ac52d947
https://hg.mozilla.org/mozilla-central/rev/0ab8644c9e71