Everywhere: Bunch of random fixes #25997
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Plus some drive-by changes, like removing redundant `Detail::` prefixes and being more studious about forwarding.
This makes my IDE happy when viewing that file
We can't really move from the NNRP as we also need to reference it in the same statement, so we pass it as a reference instead, as we are now using it only as such.
…Plan9FSMessage This is an ugly workaround, but more correct than the previous version.
otherwise we would always copy it.
github-actions
bot
added
the
👀 pr-needs-review
BertalanD
reviewed
Jun 14, 2025
| @@ -55,7 +55,6 @@ class CircularQueue { | |||
| VERIFY(!is_empty()); | |||
| auto& slot = elements()[m_head]; | |||
| T value = move(slot); | |||
| slot.~T(); | |||
There was a problem hiding this comment.
This change is incorrect; the destructors of moved-from objects are run regardless according to C++ semantics (e.g. in the case of variables on the stack).
There was a problem hiding this comment.
well clang-sa pointed this one out, but I see your point
not sure if I should mark this somehow (launder?), but I will drop this commit for now
BertalanD
reviewed
Jun 14, 2025
| construct_at<E>(&m_error, other.release_error()); | ||
| construct_at<E>(&m_error, forward<ErrorOr>(other).release_error()); |
There was a problem hiding this comment.
I don't think other is a forwarding reference here, as we're not in a template method. Does forward even do anything here?
There was a problem hiding this comment.
I think in this case it's a move?
But can make it an explicit move in case it is really needed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some found by trowing the clang static analyzer against the code,
The Error one is just my back log (thanks andrew)
The LibElf ones are just random.