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

cppcoreguidelines-rvalue-reference-param-not-moved not reported for templated method #61420

Closed
firewave opened this issue Mar 14, 2023 · 8 comments

Comments

@firewave
Copy link

firewave commented Mar 14, 2023

#include <string>

class C
{
public:
    template<typename T>
    void str(T&& s) { // no warning
        mStr = s;
    }

    void str2(std::string&& s) { // warning
        mStr = s;
    }
private:
    std::string mStr;
};

void f()
{
    C c;
    c.str("");
    c.str2("");
}
<source>:11:29: warning: rvalue reference parameter 's' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
    void str2(std::string&& s) {
                            ^

https://godbolt.org/z/zddq9hz34

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 14, 2023

@llvm/issue-subscribers-clang-tidy

@PiotrZSL
Copy link
Member

No issue here:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f18-for-will-move-from-parameters-pass-by-x-and-stdmove-the-parameter

Enforcement: Flag all X&& parameters (where X is not a template type parameter name) where the function body uses them without std::move.
If someone would use this:

    c.str(std::string());
    std::string s;
    c.str(s);

Then you got problem, because std::move could be invalid. You probably looking for some other check like one that suggest using std::forward:
F.19: For “forward” parameters, pass by TP&& and only std::forward the parameter
But currently check for F.19 is not implemented

@firewave
Copy link
Author

firewave commented Mar 18, 2023

There's already https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html which is somewhat related.

@ccotter
Copy link
Contributor

ccotter commented Apr 1, 2023

https://reviews.llvm.org/D146921 implements cppcoreguideline F.19, which flags when a forwarding reference is not forwarded with std::forward in the function body.

@PiotrZSL
Copy link
Member

Already fixed by new check.

@firewave
Copy link
Author

I assume the new check is cppcoreguidelines-missing-std-forward.
https://godbolt.org/z/5KMhn9Err

A reference to the commit fixing this would be great if closing the ticket.

@PiotrZSL
Copy link
Member

PiotrZSL commented Jul 24, 2023

Here is commit: 5902bb9 and ccotter already pointed out a review for that commit.

@firewave
Copy link
Author

Your comment made it seem like it was different from the referenced review. My bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants