[clang-tidy] Implement CppCoreGuideline F.18

[ccotter]

Summary

Warn when an rvalue reference function paramter is never moved
from within the function body.

[ccotter]

• Does not offer fixits

[njames93]

It'd be interesting to see how this handles variadic templates, I have a feeling if it isn't done correctly, there will be a lot of false positives. Can test be added to demonstrate the behaviour.
What happens with code like this

void foo(bar&& B) {
  std::move(B);
}

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

4–36

You can just declare these methods and the base struct, no need to do anything fancy in here and put definitions in there

[ccotter]

What happens with code like this

void foo(bar&& B) {
  std::move(B);
}

My new check does not flag this function, although it looks like another check flagged the move expression: ignoring return value of function declared with const attribute [clang-diagnostic-unused-value]. Guideline F.18 only says to flag functions where "the function body uses them without std::move." I think this could be improved with something like "the function body does not move construct or move assign the parameter, and does not pass the result of move() to another function which accepts rvalue references". I can open an issue on the CppCoreGuidelines repo to discuss this. On a related note, I believe there's another clang-tidy check that flags move on const values, so I could see clang-tidy similarly checking other "nonsense" move expressions such as this one (separate check).

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

4–36

Ah, I picked a different test to copy/paste from which does define them 😛. Will fix.

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

4–36

Note, I think all but one existing checks end up defining a more thorough definition of remove_reference:

git grep 'struct remove_ref' | sed -r 's/:.*//' | sort | uniq -c | sort -n
  1 modernize/use-transparent-functors.cpp
  2 cppcoreguidelines/rvalue-reference-param-not-moved.cpp
  3 bugprone/Inputs/unchecked-optional-access/absl/types/optional.h
  3 bugprone/forwarding-reference-overload.cpp
  3 misc/throw-by-value-catch-by-reference.cpp
  3 misc/unconventional-assign-operator.cpp
  3 modernize/use-emplace.cpp
  3 performance/for-range-copy.cpp
  3 performance/move-constructor-init.cpp
  4 bugprone/move-forwarding-reference.cpp
  4 bugprone/use-after-move.cpp
  4 performance/move-const-arg-const-ref.cpp
  4 performance/move-const-arg-trivially-copyable.cpp
  4 performance/move-const-arg.cpp

[ccotter]

• two more tests
• Minimize test

[ccotter]

• Match unresolved calls to move

[ccotter]

What happens with code like this

void foo(bar&& B) {
  std::move(B);
}

I raised https://github.com/isocpp/CppCoreGuidelines/issues/2026

[ccotter]

• Fix windows, fix crash

[ccotter]

• Fix windows, fix crash

[ccotter]

• Properly handle forwarding references

[ccotter]

• Allow move of any expr containing the parameter

[carlosgalvezp]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

14

Not sure if this is allowed in the repo or if we should use something equivalent from ADT.

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h

15–16

Update namespace to C++17

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

You specified C++11 in the LangOpts, but test against C++14 or later in the test, maybe change it here to c++11-or-later?

46

Could we add something like this too?

template <typename T>
struct Foo
{
  void never_moves(T&& t) {}
};

Here t is not a universal reference so the check should warn.

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

The test uses generalize lambda capture, so it needs 14. Should I split the tests that use C++14 features to a separate file, and update this test file to use 11?

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

Ah, and use of auto parameters in other lambdas. So there are a few tests that would need to be split out to a separate file, use an ifdef __cplusplus check (I see one other cppcoreguidelines test does this).

[ccotter]

• Fix universal reference check; move diagnostic location to parameter name

[ccotter]

• Use match instead of bespoke traversal

[ccotter]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

14

I was able to replace this with a cleaner solution

[ccotter]

In D141569#4048359, @njames93 wrote:

What happens with code like this

void foo(bar&& B) {
  std::move(B);
}

https://github.com/isocpp/CppCoreGuidelines/issues/2026#issuecomment-1402454584 pointed out there's another guideline, ES.56, which a "useless" move such as this example would violate. I imagine this should be a different clang-tidy check though?

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

Oops I forgot to submit my comment. Yes I think we could keep everything in one test file running c++11-or-later, and #ifdef the C++14 parts.

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

This didn't actually end up working. The test failed looking for expected CHECK-MESSAGES, e.g.,

#if __cplusplus >= 201402L
/* bad code */
// CHECK-MESSAGES
#endif

I'll go the separate file route then?

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

Hmm right.

What about 2 RUN commands in the same file? One of them you run it for c++11 and the other for c++14-or-later. You can add a check-prefix for the C++14 case, then use CHECK-MESSAGES-SOMEPREFIX such that it only applies to that test invocation. No need to duplicate CHECK-MESSAGES for both runs - if no prefix is specified, it will apply to all runs.

Something like:

#if __cplusplus >= 201402L
/* bad code */
// CHECK-MESSAGES-CXX14
#endif

If it gets too messy maybe a separate test file is the better option, I'm hoping there's not much duplication that needs to be maintaned.

[ccotter]

• Use multiple runs on the same test file

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

2

Perfect - I didn't realize the test/checker framework was this flexible. Thanks!

[ccotter]

rebase

[ccotter]

In https://github.com/isocpp/CppCoreGuidelines/issues/2026, it looks like the core guideline will not permit exceptions for code that accepts an rvalue ref parameter, and the function body moves members of the parameter. So, my moves_member_of_parameter test case would be flagged. In theory, we could add a tool option to allow someone to live more "dangerously" and have the tool accept code like moves_member_of_parameter, with the caveat that the tool (as I have written it so far) would not be smart enough to detect that all static paths move every data member of the object. In any case, I'll update my test and the tool to align exactly with the guideline.

[carlosgalvezp]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

89

Nit: the style convention in LLVM is to not use braces with one-line if/else.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

53

It would be good for the check to specify which parameter it's referring to, for example:

rvalue reference paramter 'o1' is never moved...

[ccotter]

• Add parameter name to diagnostic

[ccotter]

• Update per https://github.com/isocpp/CppCoreGuidelines/issues/2026

[ccotter]

poke - anyone mind reviewing this?

I used this tool to fix two small cases of missing move in the llvm project:
https://reviews.llvm.org/D142824
https://reviews.llvm.org/D142825

[carlosgalvezp]

LGTM thanks! Let's give a couple more days for last reviews. It probably needs a rebase since the Release Notes have got a couple more checks.

[PiotrZSL]

I run this check on project that I work for, here my comments:

False-positives (for me)

std::optional<Type>&& obj + std::move(opj.value());
std::pair<Type2, Type1>&& obj+ std::forward<std::pair<Type2, Type1>>(obj);
Type1&& factory + factory.create() [on initialization list] (create is not && method, but also is not const method, and passing value as & reference would require lvalue as input)
Type&& + member(std:move(obj.member)) [on initialization list]
Type1&& + member(obj) + when Type1 its POD
Type&& + x.emplace_back(std::move(obj.x)); y.emplace_back(std::move(obj.y)); - as function
Type&& (unnamed) + throw UnimplementedException in virtual function
X& operator=(Y&&)  - when we create locally X object and assign it via operator=(X&&), Y used only to access other data
X&& + a = std::move(obj.x), b = std::move(obj.y) (any access to obj member is wraped via std::move, but not all members are moved)
    ^ same as above but some members are just used, for example instead of move std::optional<int> we just copy value (if initialized)
    ^ same on initialization list
XYZ&& as lambda argument (explicit) + std::forward<XYZ>(obj) in lambda with "using XYZ = std::vector<..>".
X&& obj as argument + for(auto& v : obj) { something(std::move(v)); }
X&& obj as argument + for(auto& v : obj) { something(std::move(v.x)); } 
X&& obj (as function argument)+ lambda called in function [&](auto& v) { v.x = std::move(obj.x); }
std::optional<std::vector<std::uint8_t>>&& obj + xyz.push_back({other.a, other.b, std::move(obj), other.c}); - vector type has no constructor, fields initialization
container&& from + for(auto&& value : from) { if (x) { a = std::move(value); } else {dest.emplace_back(std::move(value)); } }
const Type&& obj + std::move(obj) in function
FlatMap&& obj + if (const auto it = obj.find(xyz)) { something = std::move(it->second); } + obj.clear() at end
template<X> SomeType serialize(const X&, OtherType&&, bool isSomething); (in header, private template function declaration with definition in cpp that also got warning)
template <typename U>   Type& operator=(std::shared_ptr<U>&& u) {  assign(std::forward<U>(u));   return *this; }
template<typename U> void XYZ::swap(XYZ&& other) { something.reset(); other.something.reset(); T::swap(other); (T is template parameter, for example std::map);
Map&& xyz, auto&& found = xyz.find(value); if (found != xyz.cend()) { something.merge(found->second); }
Map&& xyz, for(auto& [x,y] : xyz) { something.merge(y); }:
constructor: explicit Something(T&& value) : value(std::forward<T>(value)) {}, where T is class template argument

Overall 167 issues, half false-positive, but I already use other check for adding std::move, still had some good finding.
For some of the issues message is wrong, for example it could suggest moving some members (that were not moved) or use std::move instead of std::forward.
But for both cases probably other (new) checks could be better.

In summary:

• does not understand std::forward
• does not understand partial std::move
• does not understand ::swap and ::merge
• does not understand unused (unnamed) arguments
• some of use cases are && on purpose, just to hint user that passed value should be temporary, with const & it would accept also lvalue, such variables usually are created in place via constructor call or just initialization with {x, y, z}

I would say, do check more restricted, better is find less issues, than raising warnings on correct use-cases.

Note that I run this on top of Clang 15.

[ccotter]

Some of the examples you mentioned are indeed not compliant with guideline F.18. I have a test for at least one of the examples you gave (moves_deref_optional matches your first example). The guideline is fairly strict IMO, and I asked about this in https://github.com/isocpp/CppCoreGuidelines/issues/2026, but the response was that the guideline will remain with the requirement that the whole object needs to be moved. This would imply that the fix for cases like moves_deref_optional would be to change the parameter to be a value rather than rvalue reference of the type (and move the individual parts as desired). Example X&& obj as argument + for(auto& v : obj) { something(std::move(v.x)); } also violates F.18 since the whole object is not moved (just a subobject, although I'm a little confused about the loop also).

One example (second, std::pair<Type2, Type1>&& obj+ std::forward<std::pair<Type2, Type1>>(obj);) seems invalid or incomplete, since std::pair<Type2, Type1> is not a universal reference and shouldn't be forwarded. I couldn't follow the other forward examples. In your swap example, this code also confused me - could you clarify it (it looks like the tool would be correct since I don't see any move of the parameter other).

const Type&& obj + std::move(obj) in function - this is a good catch, I don't have a test for this, but my tool incorrectly flags this. I will fix this.

All that said, I did consider but not implement an option to the tool which would allow a less strict version of F.18 which considered moves of any expression containing the parameter to be considered a move of the object. E.g., both of my tests moves_member_of_parameter and moves_deref_optional violate F.18, although I can reasonably see code written this way with the author not desiring this check to flag such code (of course, it could be compliant with the guideline by accepting a value rather than rvalue ref of the object). I recall seeing discussion elsewhere on a phab that clang-tidy tends to add options to allow less strict interpretations of the guidelines. I think it'd be worth adding this specific option. Should I add it as default enabled or disabled (i.e., should the default be strict adherence or not)?

[carlosgalvezp]

I think we aim at having it strict as default (so someone wanting to comply with the rule "as-is" can just enable the check), and then add options to relax it.

[PiotrZSL]

You right swap case can also be invalid (rvalue not needed, there).
Anyway it it has too be strict then this can be strict.

But message could be better, for example if we don't move all fields of class, then this could warn that its not fully moved, or that some members are accessed without move., or at least is not moved as whole.
Because now it could be confusing...

void Class:set(Obj&& obj) // warning: rvalue reference parameter 'obj' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
{
    this->a = std::move(obj.a);
    this->b = std::move(obj.b);
    this->c = std::move(obj.c);
}

And then when you read "is never moved" you tap your forehead, about whats going on, you did move everything you wanted.

[ccotter]

@PiotrZSL The last forward example you have should match my does_move_auto_rvalue_ref_param test, which is not flagged by the tool. Let me know if you have any other forward cases (preferred as full self contained examples) that the tool incorrectly flags, as getting forward / universal references cases are important for this tool.

[ccotter]

• Add StrictMode option, fix const&& bug

[ccotter]

• Add back -fno-delayed-template-parsing to test

[ccotter]

Rebased as well on top of the latest release notes

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

46

Done in the AClassTemplate::never_moves test.

[ccotter]

rebase again

[ccotter]

@PiotrZSL I added an option to allow disabling the strict behavior. It should address many of your examples (e.g., moving subobjects) . Let me know if you have more feedback.

[PiotrZSL]

In D141569#4136493, @ccotter wrote:

@PiotrZSL I added an option to allow disabling the strict behavior. It should address many of your examples (e.g., moving subobjects) . Let me know if you have more feedback.

I will re-test this on Monday/Tuesday. and comeback to you if I find something.

[ccotter]

• Add back -fno-delayed-template-parsing to test

[PiotrZSL]

I don't know why but with latest version I still got false positive with std::function on Clang 15.

using Functor= std::function<... something ...>;

std::unique_ptr Class::createSomething(::Functor&& functor) {
   return std::make_unique<SomeType>(std::move(functor));
}

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

42

duplicated isDefinition and ToParam

[PiotrZSL]

Other false-positive that I still see:

void SomeClass::someVirtualFunction(SomeType&&)
{
BOOST_THROW_EXCEPTION(std::runtime_error("Unexpected call ...."));
}

It shouldnt warn about virtual functions with unnamed parameter, because this can be required by interface.

[PiotrZSL]

And on something like this I still got reproduction in production code:

template <int tagValue, typename T>
struct SomeClass
{
public:
  explicit SomeClass(T&& value) : value(std::forward<T>(value)) {}
 T value;
};

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

302

that's not always rvalue, if T would be for example int&, then this going to be lvalue...

[ccotter]

• Remove duplicated matchers

[ccotter]

In

template <int tagValue, typename T>
struct SomeClass
{
public:
  explicit SomeClass(T&& value) : value(std::forward<T>(value)) {}
 T value;
};

T is not a universal reference in the constructor, it's an rvalue reference to T. There is no deducing context, so universal references are not involved here (and, std::forward would also be incorrect here). The following would be a deducing context with a universal reference:

template <int tagValue, typename T>
struct SomeClass
{
public:
  template <class T2>
  explicit SomeClass(T2&& value) : value(std::forward<T2>(value)) {}
 T value;
};

I thought there was a clang-tidy check that flagged applying std::forward on a non-universal reference, but I don't see one.

[ccotter]

I was split on handling the case where the parameter that was never moved from was not named. For this guideline enforcement to flag all unmoved rvalue reference parameters, code that std::moves into an argument to a virtual method call could be especially confusing or dangerous with regards to whether the object was truly "moved" (move constructor/assignment invoked within the method call). E.g.,

LargeObject obj;
ClassWithVirtualMerthods* v = ...;
v->some_virtual_call(std::move(obj));

If would be confusing if some implementations of the virtual method actually moved the parameter, while others didn't. I'm inclined in this case (regardless of whether the parameter is named in a virtual overridden method) to still flag this code. Looking at the rule enforcement criteria again, the rule states "Don’t conditionally move from objects" which this case falls into.

I could see there being an option for this specific case (defaulted off), although it's very specific of a case, and for something I think the rule should be especially attentive towards. Could I get some additional feedback on this specific case?

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

302

Looking at the rule enforcement, I might have overlooked one thing:

> Flag all X&& parameters (where X is not a template type parameter name) where the function body uses them without std::move.

I think this means this code should not be flagged, even if T is an object which is cheap to move but expensive to copy (and not a reference to that type). I would like for AClassTemplate<LargeObject> to be flagged, but not AClassTemplate<LargeObject&>, so let me look into this to see if I can limit this to instantiations of the type, rather than looking at the template definition itself.

[PiotrZSL]

In D141569#4139290, @ccotter wrote:

In

template <int tagValue, typename T>
struct SomeClass
{
public:
  explicit SomeClass(T&& value) : value(std::forward<T>(value)) {}
 T value;
};

T is not a universal reference in the constructor, it's an rvalue reference to T. There is no deducing context, so universal references are not involved here (and, std::forward would also be incorrect here). The following would be a deducing context with a universal reference:

Wrong:

#include <utility>

template<typename T>
struct Wrap {

  Wrap(T&& arg) 
    : value(arg)
  {}

  T value;  
};

struct Value {};

int main()
{
    Value value;
    Wrap<Value> v1(std::move(value));
    Wrap<Value> v2(std::move(value));
    Wrap<Value&> v3(value);
    return 0;
}

If you add std::move you will get compilation error, if you add std::forward, everything will work fine. Simply Value& and && will evaluate to Value&, no rvalue reference here.

[PiotrZSL]

In D141569#4139320, @ccotter wrote:

I was split on handling the case where the parameter that was never moved from was not named. For this guideline enforcement to flag all unmoved rvalue reference parameters, code that std::moves into an argument to a virtual method call could be especially confusing or dangerous with regards to whether the object was truly "moved" (move constructor/assignment invoked within the method call). E.g.,

LargeObject obj;
ClassWithVirtualMerthods* v = ...;
v->some_virtual_call(std::move(obj));

If would be confusing if some implementations of the virtual method actually moved the parameter, while others didn't. I'm inclined in this case (regardless of whether the parameter is named in a virtual overridden method) to still flag this code. Looking at the rule enforcement criteria again, the rule states "Don’t conditionally move from objects" which this case falls into.

I could see there being an option for this specific case (defaulted off), although it's very specific of a case, and for something I think the rule should be especially attentive towards. Could I get some additional feedback on this specific case?

I'm not so sure, you may have Base class Car that would have void changeBackRightDoor(Door&&) method, and then you could have class Car5Doors and Car3Doors, in such case its easy to known that Car3Doors wouldn't implement changeBackRightDoor method, and that method wouldnt move anything, using unnamed parameters is a way to go. If you dont exclude this, then at least provide option for that, like IgnoreUnnamedParameters.

[ccotter]

• Ignore params of template type
• Add option for unnamed params

[ccotter]

> If you add std::move you will get compilation error, if you add std::forward, everything will work fine. Simply Value& and && will evaluate to Value&, no rvalue reference here.

I agree that examining the template definition alone is not correct. In your original example with SomeClass, T is not a forwarding reference since T is not deduced in the constructor, so it's misleading to use forward in this context. forward "works" and leads to the behavior you are likely seeking in your SomeClass example, but not in the usual way the most readers would expect (the spirit of the CppCoreGuidelines is to provide certain restricted ideas as recommendations, and using forward in this way violates ES.56: "Flag when std::forward is applied to other than a forwarding reference."). Usually, I'd expect to see a class template on a type to have different specializations for T being a non-reference and another for T being a reference (e.g., std::future), or only allow T to be a reference or non-reference (e.g., std::optional), but not both.

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

46

@carlosgalvezp Note I've changed the check to not flag never_moves based on more recent discussion. I do think this code should flag, although I'm a little torn. If Foo constrains T to be a reference, then Foo<Obj&>::never_moves is "OK", although I think it's still confusing as never_moves could instead accept T&.

I'll need to rethink this a bit, but I've never the test in place with a FIXME and brief comment explaining the situation.

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

46

That should read "I've left the test in place"

[PiotrZSL]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

22

and you dont need allOf

23–25

those lines should be equal to just: parmVarDecl(hasType(type(rValueReferenceType()))).bind("param"),
i'm not sure why you need additional equalsBoundNode, thats useful mainly if you would do some tricks with hasParent or something...

26–30

you could try to merge those 3:

unless(hasType(qualType(references(anyOf(isConstQualified(), templateTypeParmType(hasDeclaration(templateTypeParmDecl())), substTemplateTypeParmType())))

41

you already got constructor as second in any of, it shouldnt be matched here, so this line shouldnt be needed

47

you dont need to split things like std::move or lambda detections.

you can add this right to matcher, like:

unless(hasDescendant(lambdaExpr(hasAnyCapture(capturesVar(equalsBoundMode("param"), ...

and here you may need to add new matcher to match ByCopy))
but you dont need to check lambdas in this way, you dont need to check them anyway, they should be check as descendants of functions

104–126

unless(hasDescendant(callExpr(
      anyOf(callee(functionDecl(hasName("::std::move"))),
            callee(unresolvedLookupExpr(hasAnyDeclaration(
                namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))),
      argumentCountIs(1), hasArgument(0, moveArgumentOf(StrictMode, Param)))))

any here you could also check lambdas, if you want to ignore copies, but not should why you should do that, but you can always bind this to "move", or implement lambda checking as matcher.... unless(hasAncestor(lambdaExpr(isCapturedByCopy....

Simply as you have some tests already, try pack as much as you can into registerMatchers, because once you split this across multiple submatches in diffrent functions, then it could be hard to maintain.

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h

33–34

use bool here, or it could cause some issues, you wont save anything by using bits. class alignment is already 8.
problem is that you read bools but in storeOptions you store ints, and when dump-config will be used it will show 1/0 not true/false.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst

2

add explicit documentation about options

14

one example

[PiotrZSL]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

27–30

if with this you want to exclude parameters that depend on templates, consider looking into isTemplateInstantiation AST matcher, other option could be to get DeclContext and check isDependentContext(). You would need to check this, maybe look into other checks how its done there. Simply because if this would be for example T* &&, then it could not be detected.

[ccotter]

• Simplify matcher

[ccotter]

Simplified matchers

[ccotter]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h

33–34

Ah, I happened to see ArgumentCommentCheck.h which seems to be lone check that uses bits...

[ccotter]

• Use bool
• Combine into a single matcher

[ccotter]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

104–126

Good call. Single matcher expr is cleaner.

[ccotter]

• Finish combining into a single matcher
• Rename option and add docs

[ccotter]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

302

Thinking about this more, the enforcement spec for F.18 does apply to the SomeClass example (shown below again), since T is not a deduced parameter in the context of the constructor. Indeed, it has generated a bit of discussion which I think is the specific point of the guideline to avoid confusion around the non-trivial but important concepts of rvalue references, universal references, move/forwards, etc (and, again, I think one would usually expect to see different specializations for T vs T& and/or disallowing one of T/T& to avoid confusion to the reader or even author).

template <class T>
struct SomeClass {
    SomeClass(T&&) { ... } // T is not deduced.
};

I can add an option (although I'm a little worried now as there will be 3 options for this check, which seems like a bit much) for this with the default to adhere to the guideline as stated.

[ccotter]

• Include non-deduced template types

[ccotter]

Ok, all feedback should be addressed (thanks for the comment to consolidate into a single matcher) with appropriate options to relax the enforcement of the guideline.

[PiotrZSL]

Overall not bad, except reported things, I don't have any complains.
Number of options is not an issue.
90% of users wont use them, 10% will be happy to find them instead of dealing with NOLINT.

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

52

i thing you dont need here "hasDescendant(declRefExpr(equalsBoundNode("ref"))),"
simply because hasAncestor alone is doing a trick, if lambda is ancestor, then this call is a descendant for it.

57–58

consider:

hasType(type(rValueReferenceType())),
decl().bind("param"),
optionally(hasType(
                  qualType(references(templateTypeParmType(hasDeclaration(
                      templateTypeParmDecl().bind("template-type"))))))),

no need to duplicate parmVarDecl

62

put this unless before optionally...

65–75

looks like lambda context is visible as both operator() and as body to lambaExpr directly.
this mean that it may match twice, once as functionDecl, and once as lambdaExpr.
You can merge functionDecl with cxxConstructorDecl, just do same trick like with isMoveAssignmentOperator.

I would probably start with functionDecl, and then would try do some trick like forEachParam, but we dont have such matcher....
You missing also isDefinition() in functionDecl even that you got hasBody.

111

consider style:
if (MoveCall) return;

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst

25

maybe AllowPartialMove

[ccotter]

• More matcher simplification

[ccotter]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

65–75

I don't remember why I ever had lambda specific logic here, but it doesn't seem necessary.

[ccotter]

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

111

I left this as is as I recall seeing on other phabs a preference towards keeping the curly braces for expressions that (after taking into account line length style rules) span multiple lines. I think it helps my own readability in these situations.

[PiotrZSL]

One more thing I noticed, this check can be in conflict with performance-move-const-arg.

warning: std::move of the variable 'xyz' of the trivially-copyable type 'types::xyz' has no effect; remove std::move() [performance-move-const-arg]

so would be nice to make sure that trivially-copyable types are ignored., or add option to ignore them

[ccotter]

Trivial types should not be passed by rvalue reference, but by value per the diagram under http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#fcall-parameter-passing. I feel like adding an option to opt-out of this is missing the point of this check, and rvalue references of trivial type should just be fixed rather than adding an option to permit them.

[PiotrZSL]

In D141569#4147268, @ccotter wrote:

Trivial types should not be passed by rvalue reference, but by value per the diagram under http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#fcall-parameter-passing. I feel like adding an option to opt-out of this is missing the point of this check, and rvalue references of trivial type should just be fixed rather than adding an option to permit them.

Imaginate that such trivial type could be for example 200KB in size, copy to argument, and then from argument to for example class member could be costly. Using rvalue allows to pass non const object to some for example function.
I'm asking about option, just because other check also got such option, and if both will have one, then user can decide whatever he want to use std::move or not.
Still I agree that because we don't lose anything there, moving such trivial variable using std::move still would be fine, and at that point checking trivial variables in other check could be disabled, that's what I did in my project what I was fixing these issues.
So all is up to you.

If else, then maybe it would be good to deliver this check, if something happen then before end of release it still could be fixed.

[ccotter]

> Imaginate that such trivial type could be for example 200KB in size

This should be passed by const ref then correct (listed under "Expensive to move (e.g. big BigPOD[]" in the parameter passing guidelines) ? Are there cases where a big pod type needs to be passed by rvalue ref, without std::move where const ref cannot be used instead?

[ccotter]

• More clean option name

[ccotter]

Thanks for the name suggestion

[ccotter]

bump. I never heard back on the case where using an rvalue reference for a big pod type as opposed to const ref.

[PiotrZSL]

In D141569#4163607, @ccotter wrote:

bump. I never heard back on the case where using an rvalue reference for a big pod type as opposed to const ref.

I have such use cases in project, they not necessary POD, but more like a way to wrap many arguments into single class.
Something like this:

struct A
{
    void call();  
};

struct Parameters
{
    A subclass;
    // here bunch of other members
};

void function(Parameters&& p)
{
    p.subclass.call();
}

void test()
{
    A a;
    function({a});
}

When you change this into normal lvalue reference, then it wont compile, when you add const then it also wont compile.
In this cases I would probably used //NOLINT, so don't consider this case for this check as blocker.

For POD this issue also may happen but it's related more to the scenario when function modify input data by exploiting fact that input is not const.
In such case you may for example prepare message that would be created by using {}, then call function that would take it as rvalue, function could fill up some additional data like translation id, and send it.
By using rvalue you telling that function that it is an owner of this object now and can do anything with it, including changing it, but not necessarily need to move it.
But still this is also probably use case for NOLINT.

[PiotrZSL]

I was thinking... In theory fix for this check should be one of: use const &, use value, move rvalue.
But const & is not always in an option, in utils we got 2 functions: isOnlyUsedAsConst, isExpensiveToCopy.
So in theory if variable can be used as const, then const & can be used, if var cannot be used as const and is cheap to copy, then in theory value can be used, otherwise we deal with something intended (like partial move, or write to variable), and such cases could be +- ignored in relaxed mode.

Still, this is just something to thing, not necessarily to implement.

[PiotrZSL]

@ccotter Do we commit this, or you plan to still change something here ?

[ccotter]

Yes, I'm ready for this to be committed. I don't have commit access, but if anyone could commit this on my behalf with Chris Cotter <[e-mail]> that'd be great!

[PiotrZSL]

Rebase + Fix order of checks in ReleaseNotes

[ccotter]

Thanks!

clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

add_clang_library(clangTidyCppCoreGuidelinesModule
   ProBoundsPointerArithmeticCheck.cpp
   ProTypeConstCastCheck.cpp
   ProTypeCstyleCastCheck.cpp
   ProTypeMemberInitCheck.cpp
   ProTypeReinterpretCastCheck.cpp
   ProTypeStaticCastDowncastCheck.cpp
   ProTypeUnionAccessCheck.cpp
   ProTypeVarargCheck.cpp
+  RvalueReferenceParamNotMovedCheck.cpp
   SlicingCheck.cpp
   SpecialMemberFunctionsCheck.cpp
   VirtualClassDestructorCheck.cpp
 
   LINK_LIBS
   clangTidy
   clangTidyMiscModule
   clangTidyModernizeModule

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "ProBoundsPointerArithmeticCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeCstyleCastCheck.h"
 #include "ProTypeMemberInitCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 #include "ProTypeStaticCastDowncastCheck.h"
 #include "ProTypeUnionAccessCheck.h"
 #include "ProTypeVarargCheck.h"
+#include "RvalueReferenceParamNotMovedCheck.h"
 #include "SlicingCheck.h"
 #include "SpecialMemberFunctionsCheck.h"
 #include "VirtualClassDestructorCheck.h"
 
 namespace clang::tidy {
 namespace cppcoreguidelines {
 
 /// A module containing checks of the C++ Core Guidelines
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ProTypeReinterpretCastCheck>(
         "cppcoreguidelines-pro-type-reinterpret-cast");
     CheckFactories.registerCheck<ProTypeStaticCastDowncastCheck>(
         "cppcoreguidelines-pro-type-static-cast-downcast");
     CheckFactories.registerCheck<ProTypeUnionAccessCheck>(
         "cppcoreguidelines-pro-type-union-access");
     CheckFactories.registerCheck<ProTypeVarargCheck>(
         "cppcoreguidelines-pro-type-vararg");
+    CheckFactories.registerCheck<RvalueReferenceParamNotMovedCheck>(
+        "cppcoreguidelines-rvalue-reference-param-not-moved");
     CheckFactories.registerCheck<SpecialMemberFunctionsCheck>(
         "cppcoreguidelines-special-member-functions");
     CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing");
     CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(
         "cppcoreguidelines-c-copy-assignment-signature");
     CheckFactories.registerCheck<VirtualClassDestructorCheck>(
         "cppcoreguidelines-virtual-class-destructor");
   }

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h

+//===--- RvalueReferenceParamNotMovedCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::cppcoreguidelines {
+
+/// Warns when an rvalue reference function parameter is never moved within
+/// the function body. This check implements CppCoreGuideline F.18.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.html
+class RvalueReferenceParamNotMovedCheck : public ClangTidyCheck {
+public:
+  RvalueReferenceParamNotMovedCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool AllowPartialMove;
+  const bool IgnoreUnnamedParams;
+  const bool IgnoreNonDeducedTemplateTypes;
+};
+
+} // namespace clang::tidy::cppcoreguidelines
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp

+//===--- RvalueReferenceParamNotMovedCheck.cpp - clang-tidy ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RvalueReferenceParamNotMovedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cppcoreguidelines {
+
+namespace {
+AST_MATCHER_P(LambdaExpr, valueCapturesVar, DeclarationMatcher, VarMatcher) {
+  return std::find_if(Node.capture_begin(), Node.capture_end(),
+                      [&](const LambdaCapture &Capture) {
+                        return Capture.capturesVariable() &&
+                               VarMatcher.matches(*Capture.getCapturedVar(),
+                                                  Finder, Builder) &&
+                               Capture.getCaptureKind() == LCK_ByCopy;
+                      }) != Node.capture_end();
+}
+AST_MATCHER_P2(Stmt, argumentOf, bool, AllowPartialMove, StatementMatcher,
+               Ref) {
+  if (AllowPartialMove) {
+    return stmt(anyOf(Ref, hasDescendant(Ref))).matches(Node, Finder, Builder);
+  } else {
+    return Ref.matches(Node, Finder, Builder);
+  }
+}
+} // namespace
+
+void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) {
+  auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
+
+  StatementMatcher MoveCallMatcher =
+      callExpr(
+          anyOf(callee(functionDecl(hasName("::std::move"))),
+                callee(unresolvedLookupExpr(hasAnyDeclaration(
+                    namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))),
+          argumentCountIs(1),
+          hasArgument(
+              0, argumentOf(
+                     AllowPartialMove,
+                     declRefExpr(to(equalsBoundNode("param"))).bind("ref"))),
+          unless(hasAncestor(
+              lambdaExpr(valueCapturesVar(equalsBoundNode("param"))))))
+          .bind("move-call");
+
+  Finder->addMatcher(
+      parmVarDecl(
+          hasType(type(rValueReferenceType())), parmVarDecl().bind("param"),
+          unless(hasType(references(qualType(
+              anyOf(isConstQualified(), substTemplateTypeParmType()))))),
+          optionally(hasType(qualType(references(templateTypeParmType(
+              hasDeclaration(templateTypeParmDecl().bind("template-type"))))))),
+          anyOf(hasAncestor(cxxConstructorDecl(
+                    ToParam, isDefinition(), unless(isMoveConstructor()),
+                    optionally(hasDescendant(MoveCallMatcher)))),
+                hasAncestor(functionDecl(
+                    unless(cxxConstructorDecl()), ToParam,
+                    unless(cxxMethodDecl(isMoveAssignmentOperator())),
+                    hasBody(optionally(hasDescendant(MoveCallMatcher))))))),
+      this);
+}
+
+void RvalueReferenceParamNotMovedCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+  const auto *TemplateType =
+      Result.Nodes.getNodeAs<TemplateTypeParmDecl>("template-type");
+
+  if (!Param)
+    return;
+
+  if (IgnoreUnnamedParams && Param->getName().empty())
+    return;
+
+  const auto *Function = dyn_cast<FunctionDecl>(Param->getDeclContext());
+  if (!Function)
+    return;
+
+  if (IgnoreNonDeducedTemplateTypes && TemplateType)
+    return;
+
+  if (TemplateType) {
+    if (const FunctionTemplateDecl *FuncTemplate =
+            Function->getDescribedFunctionTemplate()) {
+      const TemplateParameterList *Params =
+          FuncTemplate->getTemplateParameters();
+      if (llvm::is_contained(*Params, TemplateType)) {
+        // Ignore forwarding reference
+        return;
+      }
+    }
+  }
+
+  const auto *MoveCall = Result.Nodes.getNodeAs<CallExpr>("move-call");
+  if (!MoveCall) {
+    diag(Param->getLocation(),
+         "rvalue reference parameter %0 is never moved from "
+         "inside the function body")
+        << Param;
+  }
+}
+
+RvalueReferenceParamNotMovedCheck::RvalueReferenceParamNotMovedCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      AllowPartialMove(Options.getLocalOrGlobal("AllowPartialMove", false)),
+      IgnoreUnnamedParams(
+          Options.getLocalOrGlobal("IgnoreUnnamedParams", false)),
+      IgnoreNonDeducedTemplateTypes(
+          Options.getLocalOrGlobal("IgnoreNonDeducedTemplateTypes", false)) {}
+
+void RvalueReferenceParamNotMovedCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowPartialMove", AllowPartialMove);
+  Options.store(Opts, "IgnoreUnnamedParams", IgnoreUnnamedParams);
+  Options.store(Opts, "IgnoreNonDeducedTemplateTypes",
+                IgnoreNonDeducedTemplateTypes);
+}
+
+} // namespace clang::tidy::cppcoreguidelines

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-unsafe-functions
   This check relies heavily on, but is not exclusive to, the functions from
   the *Annex K. "Bounds-checking interfaces"* of C11.
 
 - New :doc:`cppcoreguidelines-avoid-capture-default-when-capturing-this
   <clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this>` check.
 
   Warns when lambda specify a capture default and capture ``this``.
 
-- New :doc:`llvmlibc-inline-function-decl
-  <clang-tidy/checks/llvmlibc/inline-function-decl>` check.
-
-  Checks that all implicit and explicit inline functions in header files are
-  tagged with the ``LIBC_INLINE`` macro.
-
 - New :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines
   <clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check.
 
   Flags C++20 coroutine lambdas with non-empty capture lists that may cause
   use-after-free errors and suggests avoiding captures or ensuring the lambda
   closure object has a guaranteed lifetime.
 
+- New :doc:`cppcoreguidelines-rvalue-reference-param-not-moved
+  <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check.
+
+  Warns when an rvalue reference function parameter is never moved within
+  the function body.
+
+- New :doc:`llvmlibc-inline-function-decl
+  <clang-tidy/checks/llvmlibc/inline-function-decl>` check.
+
+  Checks that all implicit and explicit inline functions in header files are
+  tagged with the ``LIBC_INLINE`` macro.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc24-c
   <clang-tidy/checks/cert/msc24-c>` to :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` was added.
 
 - New alias :doc:`cert-msc33-c

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst

+.. title:: clang-tidy - cppcoreguidelines-rvalue-reference-param-not-moved
+
+cppcoreguidelines-rvalue-reference-param-not-moved
+==================================================
+
+Warns when an rvalue reference function parameter is never moved within
+the function body.
+
+Rvalue reference parameters indicate a parameter that should be moved with
+``std::move`` from within the function body. Any such parameter that is
+never moved is confusing and potentially indicative of a buggy program.
+
+Example:
+
+.. code-block:: c++
+
+  void logic(std::string&& Input) {
+    std::string Copy(Input); // Oops - forgot to std::move
+  }
+
+Options
+-------
+
+.. option:: AllowPartialMove
+
+   If set to `true`, the check accepts ``std::move`` calls containing any
+   subexpression containing the parameter. CppCoreGuideline F.18 officially
+   mandates that the parameter itself must be moved. Default is `false`.
+
+  .. code-block:: c++
+
+    // 'p' is flagged by this check if and only if AllowPartialMove is false
+    void move_members_of(pair<Obj, Obj>&& p) {
+      pair<Obj, Obj> other;
+      other.first = std::move(p.first);
+      other.second = std::move(p.second);
+    }
+
+    // 'p' is never flagged by this check
+    void move_whole_pair(pair<Obj, Obj>&& p) {
+      pair<Obj, Obj> other = std::move(p);
+    }
+
+.. option:: IgnoreUnnamedParams
+
+   If set to `true`, the check ignores unnamed rvalue reference parameters.
+   Default is `false`.
+
+.. option:: IgnoreNonDeducedTemplateTypes
+
+   If set to `true`, the check ignores non-deduced template type rvalue
+   reference parameters. Default is `false`.
+
+  .. code-block:: c++
+
+    template <class T>
+    struct SomeClass {
+      // Below, 'T' is not deduced and 'T&&' is an rvalue reference type.
+      // This will be flagged if and only if IgnoreNonDeducedTemplateTypes is
+      // false. One suggested fix would be to specialize the class for 'T' and
+      // 'T&' separately (e.g., see std::future), or allow only one of 'T' or
+      // 'T&' instantiations of SomeClass (e.g., see std::optional).
+      SomeClass(T&& t) { }
+    };
+
+    // Never flagged, since 'T' is a forwarding reference in a deduced context
+    template <class T>
+    void forwarding_ref(T&& t) {
+      T other = std::forward<T>(t);
+    }
+
+This check implements
+`CppCoreGuideline F.18 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f18-for-will-move-from-parameters-pass-by-x-and-stdmove-the-parameter>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `cppcoreguidelines-pro-bounds-pointer-arithmetic <cppcoreguidelines/pro-bounds-pointer-arithmetic.html>`_,
    `cppcoreguidelines-pro-type-const-cast <cppcoreguidelines/pro-type-const-cast.html>`_,
    `cppcoreguidelines-pro-type-cstyle-cast <cppcoreguidelines/pro-type-cstyle-cast.html>`_, "Yes"
    `cppcoreguidelines-pro-type-member-init <cppcoreguidelines/pro-type-member-init.html>`_, "Yes"
    `cppcoreguidelines-pro-type-reinterpret-cast <cppcoreguidelines/pro-type-reinterpret-cast.html>`_,
    `cppcoreguidelines-pro-type-static-cast-downcast <cppcoreguidelines/pro-type-static-cast-downcast.html>`_, "Yes"
    `cppcoreguidelines-pro-type-union-access <cppcoreguidelines/pro-type-union-access.html>`_,
    `cppcoreguidelines-pro-type-vararg <cppcoreguidelines/pro-type-vararg.html>`_,
+   `cppcoreguidelines-rvalue-reference-param-not-moved <cppcoreguidelines/rvalue-reference-param-not-moved.html>`_,
    `cppcoreguidelines-slicing <cppcoreguidelines/slicing.html>`_,
    `cppcoreguidelines-special-member-functions <cppcoreguidelines/special-member-functions.html>`_,
    `cppcoreguidelines-virtual-class-destructor <cppcoreguidelines/virtual-class-destructor.html>`_, "Yes"
    `darwin-avoid-spinlock <darwin/avoid-spinlock.html>`_,
    `darwin-dispatch-once-nonstatic <darwin/dispatch-once-nonstatic.html>`_, "Yes"
    `fuchsia-default-arguments-calls <fuchsia/default-arguments-calls.html>`_,
    `fuchsia-default-arguments-declarations <fuchsia/default-arguments-declarations.html>`_, "Yes"
    `fuchsia-multiple-inheritance <fuchsia/multiple-inheritance.html>`_,

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

+// RUN: %check_clang_tidy -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,CXX14 -std=c++14 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,NOSUBEXPR -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,UNNAMED -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,NONDEDUCED -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowPartialMove, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: false}]}" -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template <typename>
+struct remove_reference;
+
+template <typename _Tp> struct remove_reference { typedef _Tp type; };
+template <typename _Tp> struct remove_reference<_Tp&> { typedef _Tp type; };
+template <typename _Tp> struct remove_reference<_Tp&&> { typedef _Tp type; };
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept;
+
+template <typename _Tp>
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept;
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void just_a_declaration(Obj&& o);
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template <typename T>
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  // CHECK-MESSAGES: :[[@LINE-2]]:41: warning: rvalue reference parameter 'o2' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  Obj other{std::move(o2)};
+}
+
+void never_moves_unnamed(Obj&&) {}
+// CHECK-MESSAGES-UNNAMED: :[[@LINE-1]]:31: warning: rvalue reference parameter '' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: rvalue reference parameter 'o2' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value(Obj&& o) {
+  auto f = [o]() {
+    consumes_object(std::move(o));
+  };
+  // CHECK-MESSAGES: :[[@LINE-4]]:47: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value_nested(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  auto f = [&o]() {
+    auto f_nested = [o]() {
+      consumes_object(std::move(o));
+    };
+  };
+  auto f2 = [o]() {
+    auto f_nested = [&o]() {
+      consumes_object(std::move(o));
+    };
+  };
+  auto f3 = [o]() {
+    auto f_nested = [&o]() {
+      auto f_nested_inner = [&o]() {
+        consumes_object(std::move(o));
+      };
+    };
+  };
+  auto f4 = [&o]() {
+    auto f_nested = [&o]() {
+      auto f_nested_inner = [o]() {
+        consumes_object(std::move(o));
+      };
+    };
+  };
+}
+
+void misc_lambda_checks() {
+  auto never_moves = [](Obj&& o1) {
+    Obj other{o1};
+  };
+  // CHECK-MESSAGES: :[[@LINE-3]]:31: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+#if __cplusplus >= 201402L
+  auto never_moves_with_auto_param = [](Obj&& o1, auto& v) {
+    Obj other{o1};
+  };
+  // CHECK-MESSAGES-CXX14: :[[@LINE-3]]:47: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+#endif
+}
+
+template <typename T>
+void forwarding_ref(T&& t) {
+  t.member();
+}
+
+template <typename T>
+void forwarding_ref_forwarded(T&& t) {
+  forwarding_ref(std::forward<T>(t));
+}
+
+template <typename... Ts>
+void type_pack(Ts&&... ts) {
+  (forwarding_ref(std::forward<Ts>(ts)), ...);
+}
+
+void call_forwarding_functions() {
+  Obj o;
+
+  forwarding_ref(Obj{});
+  type_pack(Obj{});
+  type_pack(Obj{}, o);
+  type_pack(Obj{}, Obj{});
+}
+
+void moves_parameter(Obj&& o) {
+  Obj moved = std::move(o);
+}
+
+void moves_parameter_extra_parens(Obj&& o) {
+  Obj moved = std::move((o));
+}
+
+template <typename T1, typename T2>
+struct mypair {
+  T1 first;
+  T2 second;
+};
+
+void moves_member_of_parameter(mypair<Obj, Obj>&& pair) {
+  // CHECK-MESSAGES-NOSUBEXPR: :[[@LINE-1]]:51: warning: rvalue reference parameter 'pair' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj a = std::move(pair.first);
+  Obj b = std::move(pair.second);
+}
+
+template <typename T>
+struct myoptional {
+  T& operator*() &;
+  T&& operator*() &&;
+};
+
+void moves_deref_optional(myoptional<Obj>&& opt) {
+  // CHECK-MESSAGES-NOSUBEXPR: :[[@LINE-1]]:45: warning: rvalue reference parameter 'opt' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj other = std::move(*opt);
+}
+
+void moves_optional_then_deref_resulting_rvalue(myoptional<Obj>&& opt) {
+  Obj other = *std::move(opt);
+}
+
+void pass_by_lvalue_reference(Obj& o) {
+  o.member();
+}
+
+void pass_by_value(Obj o) {
+  o.member();
+}
+
+void pass_by_const_lvalue_reference(const Obj& o) {
+  o.member();
+}
+
+void pass_by_const_lvalue_reference(const Obj&& o) {
+  o.member();
+}
+
+void lambda_captures_parameter_as_reference(Obj&& o) {
+  auto f = [&o]() {
+    consumes_object(std::move(o));
+  };
+}
+
+void lambda_captures_parameter_as_reference_nested(Obj&& o) {
+  auto f = [&o]() {
+    auto f_nested = [&o]() {
+      auto f_nested2 = [&o]() {
+        consumes_object(std::move(o));
+      };
+    };
+  };
+}
+
+#if __cplusplus >= 201402L
+void lambda_captures_parameter_generalized(Obj&& o) {
+  auto f = [o = std::move(o)]() {
+    consumes_object(std::move(o));
+  };
+}
+#endif
+
+void negative_lambda_checks() {
+  auto never_moves_nested = [](Obj&& o1) {
+    auto nested = [&]() {
+      Obj other{std::move(o1)};
+    };
+  };
+
+#if __cplusplus >= 201402L
+  auto auto_lvalue_ref_param = [](auto& o1) {
+    Obj other{o1};
+  };
+
+  auto auto_forwarding_ref_param = [](auto&& o1) {
+    Obj other{o1};
+  };
+
+  auto does_move_auto_rvalue_ref_param = [](auto&& o1) {
+    Obj other{std::forward(o1)};
+  };
+#endif
+
+  auto does_move = [](Obj&& o1) {
+    Obj other{std::move(o1)};
+  };
+
+  auto not_rvalue_ref = [](Obj& o1) {
+    Obj other{std::move(o1)};
+  };
+
+  Obj local;
+  auto captures = [local]() { };
+}
+
+struct AClass {
+  void member_with_lambda_no_move(Obj&& o) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+    auto captures_this = [=, this]() {
+      Obj other = std::move(o);
+    };
+  }
+  void member_with_lambda_that_moves(Obj&& o) {
+    auto captures_this = [&, this]() {
+      Obj other = std::move(o);
+    };
+  }
+};
+
+void useless_move(Obj&& o) {
+  // FIXME - The object is not actually moved from - this should probably be
+  // flagged by *some* check. Which one?
+  std::move(o);
+}
+
+template <typename>
+class TemplatedClass;
+
+template <typename T>
+void unresolved_lookup(TemplatedClass<T>&& o) {
+  TemplatedClass<T> moved = std::move(o);
+}
+
+struct DefinesMove {
+  DefinesMove(DefinesMove&& rhs) : o(std::move(rhs.o)) { }
+  DefinesMove& operator=(DefinesMove&& rhs) {
+    if (this != &rhs) {
+      o = std::move(rhs.o);
+    }
+    return *this;
+  }
+  Obj o;
+};
+
+struct DeclaresMove {
+  DeclaresMove(DeclaresMove&& rhs);
+  DeclaresMove& operator=(DeclaresMove&& rhs);
+};
+
+struct AnotherObj {
+  AnotherObj(Obj&& o) : o(std::move(o)) {}
+  AnotherObj(Obj&& o, int) { o = std::move(o); }
+  Obj o;
+};
+
+template <class T>
+struct AClassTemplate {
+  AClassTemplate(T&& t) {}
+  // CHECK-MESSAGES-NONDEDUCED: :[[@LINE-1]]:22: warning: rvalue reference parameter 't' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  void moves(T&& t) {
+    T other = std::move(t);
+  }
+  void never_moves(T&& t) {}
+  // CHECK-MESSAGES-NONDEDUCED: :[[@LINE-1]]:24: warning: rvalue reference parameter 't' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+};
+
+void instantiate_a_class_template() {
+  Obj o;
+  AClassTemplate<Obj> withObj{std::move(o)};
+  withObj.never_moves(std::move(o));
+
+  AClassTemplate<Obj&> withObjRef(o);
+  withObjRef.never_moves(o);
+}