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.

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

src: remove unused PromiseWrap-related code #49335

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

joyeecheung
Copy link
Member

PromiseWrap has been removed in
#39135 and we do not have any internal object setting the internal field at 0 as a promise (we always set the first field as an aligned pointer to the embedder ID). As result GetAssignedPromiseWrapAsyncId() always just returns AsyncWrap::kInvalidAsyncId and turn the removed block into noops. So the block just can be removed.

PromiseWrap has been removed in
nodejs#39135 and we do not have any
internal object setting the internal field at 0 as a promise
(we always set the first field as an aligned pointer to
the embedder ID). As result GetAssignedPromiseWrapAsyncId()
always just returns AsyncWrap::kInvalidAsyncId and turn
the removed block into noops. So the block just can be removed.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 26, 2023
@joyeecheung
Copy link
Member Author

Also see the coverage report which shows that the code is unused and always just returns AsyncWrap::kInvalidAsyncId: https://app.codecov.io/gh/nodejs/node/commit/4ba73706507aebbd746838e903e38d568ca87e9d/blob/src/node_task_queue.cc#L54

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 31, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 31, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@Flarna Flarna added async_hooks Issues and PRs related to the async hooks subsystem. commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 1, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 1, 2023
@nodejs-github-bot nodejs-github-bot merged commit 295572e into nodejs:main Sep 1, 2023
40 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 295572e

UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
PromiseWrap has been removed in
#39135 and we do not have any
internal object setting the internal field at 0 as a promise
(we always set the first field as an aligned pointer to
the embedder ID). As result GetAssignedPromiseWrapAsyncId()
always just returns AsyncWrap::kInvalidAsyncId and turn
the removed block into noops. So the block just can be removed.

PR-URL: #49335
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PromiseWrap has been removed in
nodejs#39135 and we do not have any
internal object setting the internal field at 0 as a promise
(we always set the first field as an aligned pointer to
the embedder ID). As result GetAssignedPromiseWrapAsyncId()
always just returns AsyncWrap::kInvalidAsyncId and turn
the removed block into noops. So the block just can be removed.

PR-URL: nodejs#49335
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants