Index: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt @@ -8,6 +8,7 @@ GoogleTidyModule.cpp IntegerTypesCheck.cpp MemsetZeroLengthCheck.cpp + NonConstReferences.cpp OverloadedUnaryAndCheck.cpp StringReferenceMemberCheck.cpp TodoCommentCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp @@ -21,6 +21,7 @@ #include "IntegerTypesCheck.h" #include "MemsetZeroLengthCheck.h" #include "OverloadedUnaryAndCheck.h" +#include "NonConstReferences.h" #include "StringReferenceMemberCheck.h" #include "TodoCommentCheck.h" #include "UnnamedNamespaceInHeaderCheck.h" @@ -47,6 +48,8 @@ "google-runtime-int"); CheckFactories.registerCheck( "google-runtime-operator"); + CheckFactories.registerCheck( + "google-runtime-references"); CheckFactories.registerCheck( "google-runtime-member-string-references"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h +++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h @@ -0,0 +1,36 @@ +//===--- NonConstReferences.h - clang-tidy ----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NON_CONST_REFERENCES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NON_CONST_REFERENCES_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace google { +namespace runtime { + +/// \brief Checks the usage of non-constant references in function parameters. +/// +/// https://google.github.io/styleguide/cppguide.html#Reference_Arguments +class NonConstReferences : public ClangTidyCheck { +public: + NonConstReferences(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace runtime +} // namespace google +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NON_CONST_REFERENCES_H Index: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp +++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp @@ -0,0 +1,119 @@ +//===--- NonConstReferences.cpp - clang-tidy --------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "NonConstReferences.h" +#include "clang/AST/DeclBase.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace google { +namespace runtime { + +void NonConstReferences::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + parmVarDecl( + unless(isInstantiated()), + hasType(references( + qualType(unless(isConstQualified())).bind("referenced_type"))), + unless(hasType(rValueReferenceType()))) + .bind("param"), + this); +} + +void NonConstReferences::check(const MatchFinder::MatchResult &Result) { + const auto *Parameter = Result.Nodes.getNodeAs("param"); + const auto *Function = + dyn_cast_or_null(Parameter->getParentFunctionOrMethod()); + + if (Function == nullptr || Function->isImplicit()) + return; + + if (!Function->isCanonicalDecl()) + return; + + if (const CXXMethodDecl *Method = dyn_cast(Function)) { + // Don't warn on implementations of an interface using references. + if (Method->begin_overridden_methods() != Method->end_overridden_methods()) + return; + // Don't warn on lambdas, as they frequently have to conform to the + // interface defined elsewhere. + if (Method->getParent()->isLambda()) + return; + } + + auto ReferencedType = *Result.Nodes.getNodeAs("referenced_type"); + // Don't warn on function references, they shouldn't be constant. + if (ReferencedType->isFunctionProtoType()) + return; + + // Don't warn on dependent types in templates. + if (ReferencedType->isDependentType()) + return; + + if (Function->isOverloadedOperator()) { + switch (Function->getOverloadedOperator()) { + case clang::OO_LessLess: + case clang::OO_PlusPlus: + case clang::OO_MinusMinus: + case clang::OO_PlusEqual: + case clang::OO_MinusEqual: + case clang::OO_StarEqual: + case clang::OO_SlashEqual: + case clang::OO_PercentEqual: + case clang::OO_LessLessEqual: + case clang::OO_GreaterGreaterEqual: + case clang::OO_PipeEqual: + case clang::OO_CaretEqual: + case clang::OO_AmpEqual: + // Don't warn on the first parameter of operator<<(Stream&, ...), + // operator++, operator-- and operation+assignment operators. + if (Function->getParamDecl(0) == Parameter) + return; + break; + case clang::OO_GreaterGreater: { + auto isNonConstRef = [](clang::QualType T) { + return T->isReferenceType() && + !T.getNonReferenceType().isConstQualified(); + }; + // Don't warn on parameters of stream extractors: + // Stream& operator>>(Stream&, Value&); + // Both parameters should be non-const references by convention. + if (isNonConstRef(Function->getParamDecl(0)->getType()) && + (Function->getNumParams() < 2 || // E.g. member operator>>. + isNonConstRef(Function->getParamDecl(1)->getType())) && + isNonConstRef(Function->getReturnType())) + return; + break; + } + default: + break; + } + } + + // Some functions use references to comply with established standards. + if (Function->getDeclName().isIdentifier() && Function->getName() == "swap") + return; + + // iostream parameters are typically passed by non-const reference. + if (StringRef(ReferencedType.getAsString()).endswith("stream")) + return; + + diag(Parameter->getLocation(), + "non-const reference parameter '%0', make it const or use a pointer") + << Parameter->getName(); +} + +} // namespace runtime +} // namespace google +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst @@ -0,0 +1,7 @@ +google-runtime-references +========================= + +Checks the usage of non-constant references in function parameters. + +The corresponding style guide rule: +https://google.github.io/styleguide/cppguide.html#Reference_Arguments Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -38,6 +38,7 @@ google-runtime-member-string-references google-runtime-memset google-runtime-operator + google-runtime-references llvm-header-guard llvm-include-order llvm-namespace-comment Index: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp +++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp @@ -0,0 +1,139 @@ +// RUN: %check_clang_tidy %s google-runtime-references %t -- -- -std=c++11 + +int a; +int &b = a; +int *c; +void f1(int a); +void f2(int *b); +void f3(const int &c); +void f4(int const &d); + +// Don't warn on implicit operator= in c++11 mode. +class A { + virtual void f() {} +}; +// Don't warn on rvalue-references. +struct A2 { + A2(A2&&) = default; + void f(A2&&) {} +}; + +// Don't warn on iostream parameters. +namespace xxx { +class istream { }; +class ostringstream { }; +} +void g1(xxx::istream &istr); +void g1(xxx::ostringstream &istr); + +void g1(int &a); +// CHECK-MESSAGES: [[@LINE-1]]:14: warning: non-const reference parameter 'a', make it const or use a pointer [google-runtime-references] + +struct s {}; +void g2(int a, int b, s c, s &d); +// CHECK-MESSAGES: [[@LINE-1]]:31: warning: non-const reference parameter 'd', {{.*}} + +typedef int &ref; +void g3(ref a); +// CHECK-MESSAGES: [[@LINE-1]]:13: warning: non-const reference {{.*}} + +void g4(int &a, int &b, int &); +// CHECK-MESSAGES: [[@LINE-1]]:14: warning: non-const reference parameter 'a', {{.*}} +// CHECK-MESSAGES: [[@LINE-2]]:22: warning: non-const reference parameter 'b', {{.*}} +// CHECK-MESSAGES: [[@LINE-3]]:30: warning: non-const reference parameter '', {{.*}} + +class B { + B(B& a) {} +// CHECK-MESSAGES: [[@LINE-1]]:8: warning: non-const reference {{.*}} + virtual void f(int &a) {} +// CHECK-MESSAGES: [[@LINE-1]]:23: warning: non-const reference {{.*}} + void g(int &b); +// CHECK-MESSAGES: [[@LINE-1]]:15: warning: non-const reference {{.*}} + + // Don't warn on the parameter of stream extractors defined as members. + B& operator>>(int& val) { return *this; } +}; + +// Only warn on the first declaration of each function to reduce duplicate +// warnings. +void B::g(int &b) {} + +// Don't warn on the first parameter of stream inserters. +A& operator<<(A& s, int&) { return s; } +// CHECK-MESSAGES: [[@LINE-1]]:25: warning: non-const reference parameter '', {{.*}} + +// Don't warn on either parameter of stream extractors. Both need to be +// non-const references by convention. +A& operator>>(A& input, int& val) { return input; } + +// Don't warn on lambdas. +auto lambda = [] (int&) {}; + +// Don't warn on typedefs, as we'll warn on the function itself. +typedef int (*fp)(int &); + +// Don't warn on function references. +typedef void F(); +void g5(const F& func) {} +void g6(F& func) {} + +template +void g7(const T& t) {} + +template +void g8(T t) {} + +void f5() { + g5(f5); + g6(f5); + g7(f5); + g7(f5); + g8(f5); + g8(f5); +} + +// Don't warn on dependent types. +template +void g9(T& t) {} +template +void g10(T t) {} + +void f6() { + int i; + float f; + g9(i); + g9(i); + g9(i); + g10(i); + g10(f); +} + +// Warn only on the overridden methods from the base class, as the child class +// only implements the interface. +class C : public B { + C(); + virtual void f(int &a) {} +}; + +// Don't warn on operator<< with streams-like interface. +A& operator<<(A& s, int) { return s; } + +// Don't warn on swap(). +void swap(C& c1, C& c2) {} + +// Don't warn on standalone operator++, operator--, operator+=, operator-=, +// operator*=, etc. that all need non-const references to be functional. +A& operator++(A& a) { return a; } +A operator++(A& a, int) { return a; } +A& operator--(A& a) { return a; } +A operator--(A& a, int) { return a; } +A& operator+=(A& a, const A& b) { return a; } +A& operator-=(A& a, const A& b) { return a; } +A& operator*=(A& a, const A& b) { return a; } +A& operator/=(A& a, const A& b) { return a; } +A& operator%=(A& a, const A& b) { return a; } +A& operator<<=(A& a, const A& b) { return a; } +A& operator>>=(A& a, const A& b) { return a; } +A& operator|=(A& a, const A& b) { return a; } +A& operator^=(A& a, const A& b) { return a; } +A& operator&=(A& a, const A& b) { return a; }