ccotter added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:301
+ }
+ void never_moves(T&& t) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter 't'
is never moved from inside the function body
[cppcoreguidelines-rvalue-reference-param-not-moved]
----------------
ccotter wrote:
> PiotrZSL wrote:
> > that's not always rvalue, if T would be for example int&, then this going
> > to be lvalue...
> 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.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141569/new/
https://reviews.llvm.org/D141569
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
