-
-
Notifications
You must be signed in to change notification settings - Fork 9
Enable cppcoreguidelines-rvalue-reference-param-not-moved #1901
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
Enable cppcoreguidelines-rvalue-reference-param-not-moved #1901
Conversation
Signed-off-by: Balakrishna Avulapati <ba@bavulapati.com>
| // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved) | ||
| auto JSON::assign(const JSON::String &key, JSON &&value) -> void { | ||
| assert(this->is_object()); | ||
| this->data_object.emplace(key, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| this->data_object.emplace(key, value); | |
| this->data_object.emplace(key, std::move(value)); |
Maybe what it wants is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but that would conflict with another rule saying that move is ineffective
error: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg,-warnings-as-errors]
805 | this->data_object.emplace(key, std::move(value));
| ^~~~~~~~~~ ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, OK
| std::move(other.data.begin(), other.data.end(), | ||
| std::move(other.data.begin(), std::move(other).data.end(), | ||
| std::back_inserter(this->data)); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this return is needed
Signed-off-by: Balakrishna Avulapati <ba@bavulapati.com>
|
@jviotti PTAL |
|
Looks good, but there are conflicts again |
|
Hey @bavulapati , I'm not sure that are you still working on these, so let me close them to reduce noise. Always feel free to re-open if you ever get back to it |
No description provided.