Closed Bug 1535804 Opened 2 years ago Closed 8 months ago

Implement the Static class features proposal

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: alex.fdm, Assigned: anba, NeedInfo)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, parity-chrome)

Attachments

(9 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

The proposal is at stage 3, with implementation activity in other engines.

Priority: -- → P2
Duplicate of this bug: 1540228
Keywords: parity-chrome

I'm super excited for this feature. When do folks think this will be shipped?

This allows to convert two loops to use std::count_if and will also be used
in later patches. Also amend a loop to use continue to decrease the nesting
levels.

The previous approach to handle unnecessary .initializers scopes doesn't seem
to work correctly, because dis(class{constructor(){}}) shows that the scope is
still allocated. When moved into the bytecode emitter, the scope was correctly
omitted.
Also add a more descriptive comment to explain why .initializers is added in
its own scope.

Depends on D53636

  • yield is always a name when parsing a class field initialiser, therefore we
    don't need to pass YieldHandling around.
  • this is always defined when class initialisers run, therefore we don't need
    to pass HasHeritage around.
  • Also change FunctionBox::initFieldInitializer to only set those members
    which are affected by class field initialisers. In all other cases use the
    default state set through initWithEnclosingParseContext.

Depends on D53637

Parser.{h,cpp}:

  • Add a new ClassFields struct to count class fields to avoid passing
    another two size_t parameters pair to various functions.

BytecodeEmitter.{h,cpp}:

  • Use a separate enum class instead of a plain bool isStatic for readability.

ObjectEmitter.{h,cpp}:

  • Add a separate state for class field initialisation to avoid duplicating
    multiple states for instance vs. static class fields.
  • Also add an additional state to track when the initialiser expression was
    emitted to ensure emitFieldInitializerHomeObject won't be called out of order.

Depends on D53638

Depends on D53640

Class field initialisers didn't call leaveInnerFunction, which means direct
eval flags weren't properly propagated through PropagateTransitiveParseFlags.

Depends on D53641

Test262 doesn't cover these tests.

Depends on D53642

NI to make sure the review request didn't fall through the cracks. :-)

Flags: needinfo?(jorendorff)

ClassField::initializer() is never null, because when no initialiser
expression is present, we're synthesising a "raw-undefined" node. There were
plans to optimise this case, but because the current code is unfinished, it
makes more sense to remove the handling for ClassField::initializer()
returning null (for now at least).

Assignee: nobody → andrebargull
Status: NEW → ASSIGNED

Decrease the nesting level to improve the readibility a bit.

Depends on D63509

Attachment #9109646 - Attachment description: Bug 1535804 - Part 1: Add iterator_traits to ListNode iterator. r=jorendorff! → Bug 1535804 - Part 3: Add iterator_traits to ListNode iterator. r=arai!
Attachment #9109648 - Attachment description: Bug 1535804 - Part 2: Handle unnecessary .initializers scope when emitting byte code. r=jorendorff! → Bug 1535804 - Part 4: Handle unnecessary .initializers scope when emitting byte code. r=arai!
Attachment #9109649 - Attachment description: Bug 1535804 - Part 3: Remove unnecessary parameters for class fields. r=jorendorff! → Bug 1535804 - Part 5: Remove unnecessary parameters for class fields. r=arai!
Attachment #9109650 - Attachment description: Bug 1535804 - Part 4: Add 'isStatic' code paths to various functions. r=jorendorff! → Bug 1535804 - Part 6: Add 'isStatic' code paths to various functions. r=arai!
Attachment #9109651 - Attachment description: Bug 1535804 - Part 5: Parse and process static class fields. r=jorendorff! → Bug 1535804 - Part 7: Parse and process static class fields. r=arai!
Attachment #9109652 - Attachment description: Bug 1535804 - Part 6: Enable test262 tests. r=jorendorff! → Bug 1535804 - Part 8: Enable test262 tests. r=arai!
Attachment #9109654 - Attachment description: Bug 1535804 - Part 7: Add missing leaveInnerFunction call for class field initialisers. r=jorendorff! → Bug 1535804 - Part 9: Add missing leaveInnerFunction call for class field initialisers. r=arai!
Attachment #9109655 - Attachment is obsolete: true
Attachment #9109646 - Attachment description: Bug 1535804 - Part 3: Add iterator_traits to ListNode iterator. r=arai! → Bug 1535804 - Part 3: Add iterator_traits to ListNode iterator. r=jorendorff!
Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b6c1c4cde0b
Part 1: Remove unfinished code to handle non-present class field initialisers. r=arai
https://hg.mozilla.org/integration/autoland/rev/da3d3ad76c02
Part 2: Decrease nesting level in emitCreateFieldInitializers. r=arai
https://hg.mozilla.org/integration/autoland/rev/3aa03a84ae22
Part 3: Add iterator_traits to ListNode iterator. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/000cc3139baa
Part 4: Handle unnecessary .initializers scope when emitting byte code. r=arai
https://hg.mozilla.org/integration/autoland/rev/148b48f68f56
Part 5: Remove unnecessary parameters for class fields. r=arai
https://hg.mozilla.org/integration/autoland/rev/210ed28f7ec4
Part 6: Add 'isStatic' code paths to various functions. r=arai
https://hg.mozilla.org/integration/autoland/rev/2c8e0801719c
Part 7: Parse and process static class fields. r=arai
https://hg.mozilla.org/integration/autoland/rev/a40744563dc4
Part 8: Enable test262 tests. r=arai
https://hg.mozilla.org/integration/autoland/rev/1fd2c781ac64
Part 9: Add missing leaveInnerFunction call for class field initialisers. r=arai

\o/

Should we send an intent-to-ship?

Flags: needinfo?(andrebargull)

Does this bug also covers private static class fields? I can't find open bug for them. They also don't seem to work in 75 Nightly.

(In reply to Jan de Mooij [:jandem] from comment #17)

Should we send an intent-to-ship?

I've just send an intent-to-ship. It should be appear on dev-platform after moderator approval has been granted.

Flags: needinfo?(andrebargull)

(In reply to Sergey Rubanov from comment #18)

Does this bug also covers private static class fields? I can't find open bug for them. They also don't seem to work in 75 Nightly.

No, private class fields (neither instance nor static) aren't yet implemented.

You need to log in before you can comment on or make changes to this bug.