[clang-tidy] add option performance-move-const-arg.CheckMoveToConstRef
This option allows callers to disable the warning from https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html that would warn on the following ``` void f(const string &s); string s; f(std::move(s)); // ALLOWED if performance-move-const-arg.CheckMoveToConstRef=false ``` The reason people might want to disable this check, is because it allows callers to use `std::move()` or not based on local reasoning about the argument, and without having to care about how the function `f` accepts the argument. Indeed, `f` might accept the argument by const-ref today, but change to by-value tomorrow, and if the caller had moved the argument that they were finished with, the code would work as efficiently as possible regardless of how `f` accepted the parameter. Reviewed By: ymandel Differential Revision: https://reviews.llvm.org/D119370
This commit is contained in:
parent
aca355a3bb
commit
d038faea46
|
@ -37,6 +37,7 @@ static void replaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag,
|
|||
|
||||
void MoveConstArgCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
|
||||
Options.store(Opts, "CheckTriviallyCopyableMove", CheckTriviallyCopyableMove);
|
||||
Options.store(Opts, "CheckMoveToConstRef", CheckMoveToConstRef);
|
||||
}
|
||||
|
||||
void MoveConstArgCheck::registerMatchers(MatchFinder *Finder) {
|
||||
|
@ -193,7 +194,7 @@ void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
|
|||
<< (InvocationParm->getFunctionScopeIndex() + 1) << FunctionName
|
||||
<< *InvocationParmType << ExpectParmTypeName;
|
||||
}
|
||||
} else if (ReceivingExpr) {
|
||||
} else if (ReceivingExpr && CheckMoveToConstRef) {
|
||||
if ((*InvocationParmType)->isRValueReferenceType())
|
||||
return;
|
||||
|
||||
|
|
|
@ -18,16 +18,18 @@ namespace performance {
|
|||
|
||||
/// Find casts of calculation results to bigger type. Typically from int to
|
||||
///
|
||||
/// There is one option:
|
||||
/// The options are
|
||||
///
|
||||
/// - `CheckTriviallyCopyableMove`: Whether to check for trivially-copyable
|
||||
// types as their objects are not moved but copied. Enabled by default.
|
||||
// - `CheckMoveToConstRef`: Whether to check if a `std::move()` is passed
|
||||
// as a const reference argument.
|
||||
class MoveConstArgCheck : public ClangTidyCheck {
|
||||
public:
|
||||
MoveConstArgCheck(StringRef Name, ClangTidyContext *Context)
|
||||
: ClangTidyCheck(Name, Context),
|
||||
CheckTriviallyCopyableMove(
|
||||
Options.get("CheckTriviallyCopyableMove", true)) {}
|
||||
: ClangTidyCheck(Name, Context), CheckTriviallyCopyableMove(Options.get(
|
||||
"CheckTriviallyCopyableMove", true)),
|
||||
CheckMoveToConstRef(Options.get("CheckMoveToConstRef", true)) {}
|
||||
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
|
||||
return LangOpts.CPlusPlus;
|
||||
}
|
||||
|
@ -37,6 +39,7 @@ public:
|
|||
|
||||
private:
|
||||
const bool CheckTriviallyCopyableMove;
|
||||
const bool CheckMoveToConstRef;
|
||||
llvm::DenseSet<const CallExpr *> AlreadyCheckedMoves;
|
||||
};
|
||||
|
||||
|
|
|
@ -35,3 +35,8 @@ Options
|
|||
|
||||
If `true`, enables detection of trivially copyable types that do not
|
||||
have a move constructor. Default is `true`.
|
||||
|
||||
.. option:: CheckMoveToConstRef
|
||||
|
||||
If `true`, enables detection of `std::move()` passed as a const
|
||||
reference argument. Default is `true`.
|
||||
|
|
|
@ -0,0 +1,80 @@
|
|||
// RUN: %check_clang_tidy %s performance-move-const-arg %t \
|
||||
// RUN: -config='{CheckOptions: \
|
||||
// RUN: [{key: performance-move-const-arg.CheckMoveToConstRef, value: false}]}'
|
||||
|
||||
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) {
|
||||
return static_cast<typename std::remove_reference<_Tp>::type &&>(__t);
|
||||
}
|
||||
|
||||
template <typename _Tp>
|
||||
constexpr _Tp &&
|
||||
forward(typename remove_reference<_Tp>::type &__t) noexcept {
|
||||
return static_cast<_Tp &&>(__t);
|
||||
}
|
||||
|
||||
} // namespace std
|
||||
|
||||
struct TriviallyCopyable {
|
||||
int i;
|
||||
};
|
||||
|
||||
void f(TriviallyCopyable) {}
|
||||
|
||||
void g() {
|
||||
TriviallyCopyable obj;
|
||||
// Some basic test to ensure that other warnings from
|
||||
// performance-move-const-arg are still working and enabled.
|
||||
f(std::move(obj));
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable' has no effect; remove std::move() [performance-move-const-arg]
|
||||
// CHECK-FIXES: f(obj);
|
||||
}
|
||||
|
||||
class NoMoveSemantics {
|
||||
public:
|
||||
NoMoveSemantics();
|
||||
NoMoveSemantics(const NoMoveSemantics &);
|
||||
NoMoveSemantics &operator=(const NoMoveSemantics &);
|
||||
};
|
||||
|
||||
class MoveSemantics {
|
||||
public:
|
||||
MoveSemantics();
|
||||
MoveSemantics(MoveSemantics &&);
|
||||
|
||||
MoveSemantics &operator=(MoveSemantics &&);
|
||||
};
|
||||
|
||||
void callByConstRef1(const NoMoveSemantics &);
|
||||
void callByConstRef2(const MoveSemantics &);
|
||||
|
||||
void moveToConstReferencePositives() {
|
||||
NoMoveSemantics a;
|
||||
|
||||
// This call is now allowed since CheckMoveToConstRef is false.
|
||||
callByConstRef1(std::move(a));
|
||||
|
||||
MoveSemantics b;
|
||||
|
||||
// This call is now allowed since CheckMoveToConstRef is false.
|
||||
callByConstRef2(std::move(b));
|
||||
}
|
Loading…
Reference in a new issue