GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: avoid Isolate::GetCurrent() for platform implementation by addaleax · Pull Request #32269 · nodejs/node

Conversation

addaleax commented Mar 14, 2020

Copy link

Copy Markdown

Member

There’s no need to use Isolate::GetCurrent(), which is generally
discouraged, as the Isolate* pointer can generally be looked up
from the per-Isolate platform data structure.

Checklist

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and himself65 reacted with thumbs up emoji

All reactions

👍 2 reactions


          src: avoid Isolate::GetCurrent() for platform implementation

8cf7a3f

There’s no need to use `Isolate::GetCurrent()`, which is generally
discouraged, as the `Isolate*` pointer can generally be looked up
from the per-Isolate platform data structure.

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label

Mar 14, 2020

addaleax added the v8 platform Issues and PRs related to the Node.js implementation of v8::Platform. label

Mar 14, 2020

cjihrig approved these changes

Mar 14, 2020

View reviewed changes

devsnek approved these changes

Mar 14, 2020

View reviewed changes

gireeshpunathil reviewed

Mar 15, 2020

View reviewed changes

Comment thread

src/node_platform.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

src/node_platform.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

gireeshpunathil approved these changes

Mar 15, 2020

View reviewed changes

addaleax added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Mar 15, 2020

HarshithaKP added a commit to HarshithaKP/node that referenced this pull request

Mar 16, 2020


          src: replace handle dereference with ContainerOf

3e522af

this was influenced by nodejs#32269

HarshithaKP mentioned this pull request

Mar 16, 2020

src: replace handle dereference with ContainerOf macro #32298

Closed

2 of 4 tasks

jasnell approved these changes

Mar 16, 2020

View reviewed changes

nodejs-github-bot commented Mar 20, 2020

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29949/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell pushed a commit that referenced this pull request

Mar 21, 2020


          src: avoid Isolate::GetCurrent() for platform implementation

d129e0c

There’s no need to use `Isolate::GetCurrent()`, which is generally
discouraged, as the `Isolate*` pointer can generally be looked up
from the per-Isolate platform data structure.

PR-URL: #32269
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>

jasnell commented Mar 21, 2020

Copy link

Copy Markdown

Member

Landed in d129e0c

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell closed this

Mar 21, 2020

MylesBorins pushed a commit that referenced this pull request

Mar 24, 2020


          src: avoid Isolate::GetCurrent() for platform implementation

bd55a9a

There’s no need to use `Isolate::GetCurrent()`, which is generally
discouraged, as the `Isolate*` pointer can generally be looked up
from the per-Isolate platform data structure.

PR-URL: #32269
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>

MylesBorins mentioned this pull request

Mar 24, 2020

v13.12.0 proposal #32376

Merged

addaleax pushed a commit that referenced this pull request

Mar 29, 2020


          src: replace handle dereference with ContainerOf

768b0f5

this was influenced by #32269

PR-URL: #32298
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>

addaleax pushed a commit that referenced this pull request

Mar 30, 2020


          src: replace handle dereference with ContainerOf

06bff18

this was influenced by #32269

PR-URL: #32298
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos added the backport-blocked-v12.x label

Apr 22, 2020

targos pushed a commit that referenced this pull request

Apr 22, 2020


          src: replace handle dereference with ContainerOf

this was influenced by #32269

PR-URL: #32298
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos removed author ready

PRs with CI started, the required approvals, and no outstanding review comments.

backport-blocked-v12.x labels

Apr 25, 2020

targos pushed a commit to targos/node that referenced this pull request

Apr 25, 2020


          src: avoid Isolate::GetCurrent() for platform implementation

77a66d0

There’s no need to use `Isolate::GetCurrent()`, which is generally
discouraged, as the `Isolate*` pointer can generally be looked up
from the per-Isolate platform data structure.

PR-URL: nodejs#32269
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request

Apr 28, 2020


          src: avoid Isolate::GetCurrent() for platform implementation

8f7f4e5

There’s no need to use `Isolate::GetCurrent()`, which is generally
discouraged, as the `Isolate*` pointer can generally be looked up
from the per-Isolate platform data structure.

PR-URL: #32269
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request

May 2, 2020

v12.17.0 release proposal #33197

Merged

Reviewers

jasnell

jasnell approved these changes

cjihrig

cjihrig approved these changes

gireeshpunathil

gireeshpunathil approved these changes

+1 more reviewer

devsnek

devsnek approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++

Issues and PRs that require attention from people who are familiar with C++.

v8 platform

Issues and PRs related to the Node.js implementation of v8::Platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.