sukraat91 updated this revision to Diff 291326.
sukraat91 added a comment.
Revert to older commit.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87540/new/
https://reviews.llvm.org/D87540
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
@@ -29,6 +29,36 @@
const_iterator end() const;
};
+// Mocking std::move() and std::remove_reference<T> (since move() relies on that)
+// Since the regression tests run with -nostdinc++, standard library utilities have
+// to be mocked. Over here the mocking is required for tests that have function arguments
+// being moved.
+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);
+}
+
+} // namespace std
+
// This class simulates std::pair<>. It is trivially copy constructible
// and trivially destructible, but not trivially copy assignable.
class SomewhatTrivial {
@@ -55,6 +85,16 @@
~ExpensiveMovableType();
};
+template <typename Arg>
+struct UsesExpensiveToCopyType {
+ Arg arg;
+ ExpensiveToCopyType expensiveType;
+
+ UsesExpensiveToCopyType() = default;
+ UsesExpensiveToCopyType(ExpensiveToCopyType eType) : expensiveType{std::move(eType)} {}
+ UsesExpensiveToCopyType(ExpensiveToCopyType eType, Arg t) : expensiveType{std::move(eType)}, arg{std::move(t)} {}
+};
+
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
// CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
@@ -166,6 +206,20 @@
Obj.nonConstMethod();
}
+void negativeNoConstRefSinceMoved(ExpensiveToCopyType arg) {
+ auto F = std::move(arg);
+}
+
+template <typename T>
+T *negativeNoConstRefSinceTypeMoved(ExpensiveToCopyType t) {
+ return new T(std::move(t));
+}
+
+template <typename T>
+UsesExpensiveToCopyType<T> negativeCreate(ExpensiveToCopyType eType) {
+ return UsesExpensiveToCopyType<T>(std::move(eType));
+}
+
struct PositiveValueUnusedConstructor {
PositiveValueUnusedConstructor(ExpensiveToCopyType Copy) {}
// CHECK-MESSAGES: [[@LINE-1]]:54: warning: the parameter 'Copy'
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -63,6 +63,31 @@
return false;
}
+bool isPassedToStdMove(const ParmVarDecl &Param, ASTContext &Context) {
+ // Check if the parameter has a name, in case of functions like -
+ // void func(const ExpensiveToCopyType)
+ // {
+ //
+ // }
+ // The function having an empty body will still have a FunctionDecl and its
+ // parmVarDecl picked up by this checker. It will be an empty string and will
+ // lead to an assertion failure when using hasName(std::string) being used
+ // in the matcher below. If empty then exit indicating no move calls present
+ // for the function argument being examined.
+ const auto paramName = Param.getName();
+
+ if (paramName.empty()) {
+ return false;
+ }
+ auto Matches = match(
+ callExpr(
+ callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
+ hasArgument(0, declRefExpr(to(parmVarDecl(hasName(paramName)))))),
+ Context);
+
+ return !Matches.empty();
+}
+
} // namespace
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@@ -103,6 +128,9 @@
if (Analyzer.isMutated(Param))
return;
+ if (isPassedToStdMove(*Param, *Result.Context))
+ return;
+
const bool IsConstQualified =
Param->getType().getCanonicalType().isConstQualified();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
