clang-tidy: Fix performance-*move* warnings in headers
#26707
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
hebasto
force-pushed
the
221215-tidy-move
branch
from
f1f7472 to
22eb1b4
Compare
hebasto
changed the title
performance-move-const-arg in headersperformance-*move* warnings in headers
|
Updated f1f7472 -> 22eb1b4 (pr26707.01 -> pr26707.02):
|
| assert(constructed.size() == 1); | ||
| assert(constructed[0]->ScriptSize() == script_size); | ||
| if (in.size() > 0) return {}; | ||
| const NodeRef<Key> tl_node = std::move(constructed.front()); |
There was a problem hiding this comment.
TIL. My understanding is that without this patch tl_node would not be moved a couple lines below when returning from the function. Making it not const fixes this.
If my understanding is correct, ACK.
There was a problem hiding this comment.
To be precise, this patch makes a compiler to call a move-constructor instead of a copy-constructor in the return statement.
There was a problem hiding this comment.
NodeRef is std::shared_ptr<bla>, so this shouldn't change the compiled binary except for a removed std::atomic add and sub.
hebasto
force-pushed
the
221215-tidy-move
branch
from
22eb1b4 to
70e195a
Compare
|
The diff in #26707 (comment) does not apply |
Sorry. It should be a reversal one. Fixed. |
hebasto
force-pushed
the
221215-tidy-move
branch
from
70e195a to
1308b83
Compare
|
Rebased 70e195a -> 1308b83 (pr26707.03 -> pr26707.04) on top of the merged #26706. |
… headers 1308b83 clang-tidy: Fix `performance-no-automatic-move` in headers (Hennadii Stepanov) 0a5dc03 clang-tidy: Fix `performance-move-const-arg` in headers (Hennadii Stepanov) Pull request description: Split from bitcoin#26705 as was requested in bitcoin#26705 (comment). To test this PR, consider applying a diff as follows: ```diff --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,16 +1,7 @@ Checks: ' -*, -bugprone-argument-comment, -bugprone-use-after-move, -misc-unused-using-decls, -modernize-use-default-member-init, -modernize-use-nullptr, -performance-for-range-copy, performance-move-const-arg, performance-no-automatic-move, -performance-unnecessary-copy-initialization, -readability-redundant-declaration, -readability-redundant-string-init, ' WarningsAsErrors: ' bugprone-argument-comment, @@ -28,4 +19,4 @@ readability-redundant-string-init, CheckOptions: - key: performance-move-const-arg.CheckTriviallyCopyableMove value: false -HeaderFilterRegex: './qt' +HeaderFilterRegex: '.' ``` ACKs for top commit: fanquake: ACK 1308b83 Tree-SHA512: b7ef9a3e789846130ab4c3fd6fbe8d887bdbcd438e4cbc78e2b1ac01f819ae13d7f69c2a25f480bd36e3e7f58886a7d5a8609a3c3275c315e0697cd4010474bd
5 participants
Split from #26705 as was requested in #26705 (comment).
To test this PR, consider applying a diff as follows: