jmarrec updated this revision to Diff 365779.
jmarrec added a comment.
Forgot to run clang format on the changes
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107900/new/
https://reviews.llvm.org/D107900
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s misc-pod-const-ref-to-value %t
+
+int f1(const int &i);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);
+int f2(const int &i, double d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f2(int i, double d);
+
+int f3(double d, const int &i);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f3(double d, int i);
+
+int f4(const double &d, const double &d2);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: argument 'd2' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f4(double d, double d2);
+
+// clang-format off
+int f5(const int& i);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f5(int i);
+// clang-format on
+
+namespace n1 {
+class A {
+ static int g(const double &d);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+ // CHECK-FIXES: static int g(double d);
+};
+
+int A::g(const double &d) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+ // CHECK-FIXES: int A::g(double d) {
+ return static_cast<int>(d);
+}
+} // namespace n1
+
+// Not triggering the check
+
+int f5(int i);
+int f6(int i, double d);
+int f7(int &i); // Might be used for return...
+
+namespace n2 {
+class A {
+ static int g(double d);
+};
+
+int A::g(double d) {
+ return static_cast<int>(d);
+}
+
+class B {
+ static int g(double &d);
+};
+
+int B::g(double &d) {
+ return static_cast<int>(d);
+}
+} // namespace n2
+
+template <typename... Args>
+void f(Args &&...args);
+
+struct Widget {
+ int a[1000];
+};
+void f(const Widget &);
+
+template <class T>
+struct Max {
+ static T max(const T &a, const T &b);
+};
+int x = Max<int>::max(1, 2); // passes `int` by const reference, but this is fine
Index: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-pod-const-ref-to-value
+
+misc-pod-const-ref-to-value
+===========================
+
+This check detects when ``trivially_copyable`` types are passed by const-reference
+to a function and changes that to by value
+
+For example in the following code, it is replaced by ``void f(int i, double d)``
+
+.. code-block:: c++
+
+ void f(const int& i, const double& d);
+
+
+Options
+-------
+
+.. option:: RestrictToBuiltInTypes
+
+ If set to `true`, this check will limit itself to the `builtinType()` types. Default is `false`.
+
+.. option:: MaxSize
+
+ MaxSize: int, default 16. Above this size, passing by const-reference makes sense
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
`misc-no-recursion <misc-no-recursion.html>`_,
`misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
`misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
+ `misc-pod-const-ref-to-value <misc-pod-const-ref-to-value.html>`_, "Yes"
`misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
`misc-static-assert <misc-static-assert.html>`_, "Yes"
`misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
New checks
^^^^^^^^^^
+- New :doc:`misc-pod-const-ref-to-value
+ <clang-tidy/checks/misc-pod-const-ref-to-value>` check.
+
+ Finds trivially_copyable types are passed by const-reference to a function
+ and changes that to by value
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
@@ -0,0 +1,45 @@
+//===--- PodConstRefToValueCheck.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_MISC_PODCONSTREFTOVALUECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PODCONSTREFTOVALUECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// The check detects when trivially_copyable types are passed by
+/// const-reference to a function and changes that to by value
+//
+// RestrictToBuiltInTypes: if true (default false), restricts the check to the
+// built-in types (int, double, etc)
+//
+// MaxSize: int, default 16. Above this size, passing by const-reference makes
+// sense
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-pod-const-ref-to-value.html
+class PodConstRefToValueCheck : public ClangTidyCheck {
+public:
+ PodConstRefToValueCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const bool RestrictToBuiltInTypes;
+ const int MaxSize = 16;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PODCONSTREFTOVALUECHECK_H
Index: clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
@@ -0,0 +1,132 @@
+//===--- PodConstRefToValueCheck.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 "PodConstRefToValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+bool isSharedPtr(const QualType &T) {
+ if (auto *R = T->getAsCXXRecordDecl())
+ return R->getQualifiedNameAsString() == "std::shared_ptr" ||
+ R->getQualifiedNameAsString() == "boost::shared_ptr";
+ return false;
+}
+
+SourceRange getTypeRange(const ParmVarDecl &Param) {
+ return SourceRange(Param.getBeginLoc(),
+ Param.getLocation().getLocWithOffset(-1));
+}
+
+PodConstRefToValueCheck::PodConstRefToValueCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ RestrictToBuiltInTypes(Options.get("RestrictToBuiltInTypes", false)),
+ MaxSize(16) {}
+
+void PodConstRefToValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "RestrictToBuiltInTypes", RestrictToBuiltInTypes);
+ Options.store(Opts, "MaxSize", MaxSize);
+}
+
+void PodConstRefToValueCheck::registerMatchers(MatchFinder *Finder) {
+ if (RestrictToBuiltInTypes) {
+ Finder->addMatcher(
+ functionDecl(
+ allOf(has(typeLoc(forEach(
+ parmVarDecl(hasType(referenceType(pointee(qualType(
+ isConstQualified(), builtinType())))))
+ .bind("param")))),
+ unless(ast_matchers::isTemplateInstantiation())))
+ .bind("func"),
+ this);
+ } else {
+ Finder->addMatcher(
+ functionDecl(allOf(has(typeLoc(forEach(
+ parmVarDecl(hasType(referenceType(pointee(
+ qualType(isConstQualified())))))
+ .bind("param")))),
+ unless(ast_matchers::isTemplateInstantiation())))
+ .bind("func"),
+ this);
+ }
+}
+
+void PodConstRefToValueCheck::check(const MatchFinder::MatchResult &Result) {
+
+ const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ const auto *ParamDecl = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+
+ // if (!ParamDecl->getType().isLocalConstQualified()) {
+ // return;
+ // }
+
+ const QualType &T = ParamDecl->getType().getNonReferenceType();
+
+ // If the parameter is NOT trivial to copy, const-ref makes sense
+ if (!T.isTriviallyCopyableType(*Result.Context)) {
+ return;
+ }
+
+ // Skip shared_ptr
+ if (isSharedPtr(T)) {
+ return;
+ }
+
+ // Skip types whose size is unknown or greater than MaxSize
+ Optional<CharUnits> Size = Result.Context->getTypeSizeInCharsIfKnown(T);
+ if (!Size.hasValue() || Size->getQuantity() > MaxSize) {
+ return;
+ }
+
+ auto Diag = diag(ParamDecl->getBeginLoc(),
+ "argument %0 is a trivially copyable type and should not be "
+ "passed by const-reference but by value");
+ if (ParamDecl->getName().empty()) {
+ for (unsigned int I = 0; I < FuncDecl->getNumParams(); ++I) {
+ if (ParamDecl == FuncDecl->getParamDecl(I)) {
+ Diag << (I + 1);
+ break;
+ }
+ }
+ } else {
+ Diag << ParamDecl;
+ }
+
+ // CharSourceRange FileRange = Lexer::makeFileCharRange(
+ // CharSourceRange::getTokenRange(getTypeRange(*ParamDecl)),
+ //*Result.SourceManager, getLangOpts());
+
+ // if (!FileRange.isValid()) {
+ // return;
+ //}
+
+ // auto Tok = tidy::utils::lexer::getQualifyingToken(
+ // tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
+ // if (!Tok)
+ // return;
+
+ Diag << FixItHint::CreateReplacement(
+ getTypeRange(*ParamDecl),
+ ParamDecl->getType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getAsString(Result.Context->getPrintingPolicy()) + " ");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -15,6 +15,7 @@
#include "NoRecursionCheck.h"
#include "NonCopyableObjects.h"
#include "NonPrivateMemberVariablesInClassesCheck.h"
+#include "PodConstRefToValueCheck.h"
#include "RedundantExpressionCheck.h"
#include "StaticAssertCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
@@ -41,6 +42,8 @@
"misc-non-copyable-objects");
CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
"misc-non-private-member-variables-in-classes");
+ CheckFactories.registerCheck<PodConstRefToValueCheck>(
+ "misc-pod-const-ref-to-value");
CheckFactories.registerCheck<RedundantExpressionCheck>(
"misc-redundant-expression");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -11,6 +11,7 @@
NoRecursionCheck.cpp
NonCopyableObjects.cpp
NonPrivateMemberVariablesInClassesCheck.cpp
+ PodConstRefToValueCheck.cpp
RedundantExpressionCheck.cpp
StaticAssertCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
