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: use unique_ptr in platform implementation #16970

Closed
wants to merge 1 commit into from

Conversation

fhinkel
Copy link
Member

@fhinkel fhinkel commented Nov 12, 2017

Replace raw pointers in task queues with std::unique_ptr. This
makes ownership clearer.

The future V8 platform interface uses smart pointers, so I figured we could change
the implementation proactively (and replace more raw pointers in other commits). What do you think?

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

src

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 12, 2017
// In the long run, that functionality is probably going to be available
// anyway, though.
// Right now, there is no way to drain only background tasks associated
// with a specific isolate, so this sometimes does more work than
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra space after "with"

Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.
@fhinkel
Copy link
Member Author

fhinkel commented Nov 13, 2017

@fhinkel
Copy link
Member Author

fhinkel commented Nov 14, 2017

VI failures look unrelated.

Thanks for the reviews. Landed in 8101042.

@fhinkel fhinkel closed this Nov 14, 2017
fhinkel added a commit to fhinkel/node that referenced this pull request Nov 14, 2017
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.

PR-URL: nodejs#16970
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
fhinkel added a commit that referenced this pull request Nov 17, 2017
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 11, 2017
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.

PR-URL: #16970
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
addaleax pushed a commit to addaleax/node that referenced this pull request May 22, 2018
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.

PR-URL: nodejs#16970
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 14, 2018
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.

Backport-PR-URL: #20901
PR-URL: #16970
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
rvagg pushed a commit that referenced this pull request Aug 16, 2018
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.

Backport-PR-URL: #20901
PR-URL: #16970
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@addaleax addaleax added the v8 platform Issues and PRs related to Node's v8::Platform implementation. label Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants