Skip to content

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 24, 2020

It is not possible for a ECMAScript class to have a prototype that isn't
either a function or null. Test coverage statistics seem to bear out
that the code removed here is unreachable.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

It is not possible for a ECMAScript class to have a prototype that isn't
either a function or null. Test coverage statistics seem to bear out
that the code removed here is unreachable.
@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Dec 24, 2020
@Trott
Copy link
Member Author

Trott commented Dec 24, 2020

It is not possible for a ECMAScript class to have a prototype that isn't
either a function or null.

@ljharb @BridgeAR If I'm wrong and there's some clever way to monkeypatch things to make this code reachable, I'd love to be corrected.

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to reach the code by setting a new prototype:

class A {}
Object.setPrototypeOf(A, Map.prototype)

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 25, 2020
@Trott
Copy link
Member Author

Trott commented Dec 25, 2020

It's possible to reach the code by setting a new prototype:

class A {}
Object.setPrototypeOf(A, Map.prototype)

Whoops, I definitely should have been able to figure that out. Thanks.

@Trott Trott closed this Dec 25, 2020
@Trott Trott deleted the class-proto branch December 25, 2020 00:16
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2020
Refs: nodejs#36622 (review)

PR-URL: nodejs#36625
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@richardlau richardlau removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Refs: #36622 (review)

PR-URL: #36625
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
Refs: #36622 (review)

PR-URL: #36625
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants