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

quic: clear clang warning #34335

Closed
wants to merge 1 commit into from
Closed

quic: clear clang warning #34335

wants to merge 1 commit into from

Conversation

gengjiawen
Copy link
Member 

[2945/2986] CXX obj/src/quic/libnode.node_quic_buffer.o
../../src/quic/node_quic_buffer.cc:160:7: warning: moving a temporary object prevents copy elision [-Wpessimizing-move]
std::move([this](size_t len) { Seek(len); }));
^
../../src/quic/node_quic_buffer.cc:160:7: note: remove std::move call here
std::move([this](size_t len) { Seek(len); }));
^~~~~~~~~~ ~
1 warning generated.
[2986/2986] LINK node

@gengjiawen gengjiawen requested a review from a team July 13, 2020 12:53
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. quic Issues and PRs related to the QUIC implementation / HTTP/3. labels Jul 13, 2020
@addaleax
Copy link
Member

Fwiw, you can also remove the std::move on L156, it doesn’t do anything.

@gengjiawen gengjiawen mentioned this pull request Jul 13, 2020
Closed
@gengjiawen gengjiawen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 13, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@addaleax
Copy link
Member

Landed in fbc38ee

addaleax pushed a commit that referenced this pull request Jul 15, 2020
@gengjiawen @addaleax
quic: clear clang warning
fbc38ee 
PR-URL: #34335
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax closed this Jul 15, 2020
@ruyadorno
Copy link
Member

This PR is related to QUIC, adding the backport-requested-v14.x label so that it could be included in a future backport.

@richardlau
Copy link
Member

This PR is related to QUIC, adding the backport-requested-v14.x label so that it could be included in a future backport.

Most of the quic PRs have dont-land-on-v14.x labels: https://github.com/nodejs/node/issues?q=label%3Aquic+is%3Aclosed
¯\_(ツ)_/¯

cjihrig pushed a commit that referenced this pull request Jul 23, 2020
@gengjiawen @cjihrig
quic: clear clang warning
08e4a2c 
PR-URL: #34335
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gengjiawen gengjiawen deleted the bugfix/clang branch July 23, 2020 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. quic Issues and PRs related to the QUIC implementation / HTTP/3.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gengjiawen @addaleax @nodejs-github-bot @ruyadorno @richardlau @jasnell