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 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( "misc-non-private-member-variables-in-classes"); + CheckFactories.registerCheck( + "misc-pod-const-ref-to-value"); CheckFactories.registerCheck( "misc-redundant-expression"); CheckFactories.registerCheck("misc-static-assert"); Index: clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h @@ -0,0 +1,41 @@ +//===--- 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) +/// +/// 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; +}; + +} // 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,108 @@ +//===--- 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 { + +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)) {} + +void PodConstRefToValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "RestrictToBuiltInTypes", RestrictToBuiltInTypes); +} + +void PodConstRefToValueCheck::registerMatchers(MatchFinder *Finder) { + if (RestrictToBuiltInTypes) { + Finder->addMatcher( + functionDecl(has(typeLoc(forEach( + parmVarDecl(hasType(referenceType(pointee(qualType( + isConstQualified(), builtinType()))))) + .bind("param"))))) + .bind("func"), + this); + } else { + Finder->addMatcher( + functionDecl( + has(typeLoc(forEach(parmVarDecl(hasType(referenceType(pointee( + qualType(isConstQualified()))))) + .bind("param"))))) + .bind("func"), + this); + } +} + +void PodConstRefToValueCheck::check(const MatchFinder::MatchResult &Result) { + + const auto *FuncDecl = Result.Nodes.getNodeAs("func"); + const auto *ParamDecl = Result.Nodes.getNodeAs("param"); + + // if (!ParamDecl->getType().isLocalConstQualified()) { + // return; + // } + + // If the parameter is NOT trivial to copy, const-ref makes sense + if (!ParamDecl->getType().getNonReferenceType().isTriviallyCopyableType( + *Result.Context)) { + 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/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 + ` 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/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-non-copyable-objects `_, `misc-non-private-member-variables-in-classes `_, + `misc-pod-const-ref-to-value `_, "Yes" `misc-redundant-expression `_, "Yes" `misc-static-assert `_, "Yes" `misc-throw-by-value-catch-by-reference `_, 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,6 @@ +.. title:: clang-tidy - misc-pod-const-ref-to-value + +misc-pod-const-ref-to-value +=========================== + +FIXME: Describe what patterns does the check detect and why. Give examples. 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,55 @@ +// 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); + +namespace n1 { +class A { + static int g(const double &d); + // CHECK-MESSAGES: :[[@LINE-1]]:18: 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]]:12: 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(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(d); +} + +class B { + static int g(double &d); +}; + +int B::g(double &d) { + return static_cast(d); +} +} // namespace n2