Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

addaleax
Copy link
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

@addaleax
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 nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 14, 2020
@addaleax addaleax added the v8 platform Issues and PRs related to Node's v8::Platform implementation. label Mar 14, 2020
gireeshpunathil
gireeshpunathil reviewed Mar 15, 2020
View reviewed changes
src/node_platform.h Show resolved Hide resolved
src/node_platform.h Show resolved Hide resolved
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 15, 2020
HarshithaKP added a commit to HarshithaKP/node that referenced this pull request Mar 16, 2020
@HarshithaKP
src: replace handle dereference with ContainerOf
3e522af 
this was influenced by nodejs#32269
@HarshithaKP HarshithaKP mentioned this pull request Mar 16, 2020
Closed
4 tasks
@nodejs-github-bot
Copy link
Collaborator

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

jasnell pushed a commit that referenced this pull request Mar 21, 2020
@addaleax @jasnell
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
Copy link
Member

jasnell commented Mar 21, 2020

Landed in d129e0c

@jasnell jasnell closed this Mar 21, 2020
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
@addaleax @MylesBorins
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 MylesBorins mentioned this pull request Mar 24, 2020
Merged
addaleax pushed a commit that referenced this pull request Mar 29, 2020
@HarshithaKP @addaleax
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
@HarshithaKP @addaleax
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 pushed a commit that referenced this pull request Apr 22, 2020
@HarshithaKP @targos
src: replace handle dereference with ContainerOf
4155358 
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 targos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@addaleax @targos
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
@addaleax @targos
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 targos mentioned this pull request May 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@devsnek devsnek devsnek approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

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 Node's v8::Platform implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@addaleax @nodejs-github-bot @jasnell @cjihrig @devsnek @gireeshpunathil @targos