[clang-tools-extra] r259530 - [clang-tidy] Add non-constant references in function parameters check.

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 2 09:27:02 PST 2016


Author: alexfh
Date: Tue Feb  2 11:27:01 2016
New Revision: 259530

URL: http://llvm.org/viewvc/llvm-project?rev=259530&view=rev
Log:
[clang-tidy] Add non-constant references in function parameters check.

Summary: This is implemented originally by Alexander Kornienko.

Reviewers: alexfh

Subscribers: cfe-commits


Patch by Haojian Wu!

Differential Revision: http://reviews.llvm.org/D16717

Added:
    clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
    clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst
    clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
    clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=259530&r1=259529&r2=259530&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Tue Feb  2 11:27:01 2016
@@ -8,6 +8,7 @@ add_clang_library(clangTidyGoogleModule
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   MemsetZeroLengthCheck.cpp
+  NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp
   StringReferenceMemberCheck.cpp
   TodoCommentCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=259530&r1=259529&r2=259530&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Tue Feb  2 11:27:01 2016
@@ -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 @@ public:
         "google-runtime-int");
     CheckFactories.registerCheck<runtime::OverloadedUnaryAndCheck>(
         "google-runtime-operator");
+    CheckFactories.registerCheck<runtime::NonConstReferences>(
+        "google-runtime-references");
     CheckFactories.registerCheck<runtime::StringReferenceMemberCheck>(
         "google-runtime-member-string-references");
     CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>(

Added: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp?rev=259530&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.cpp Tue Feb  2 11:27:01 2016
@@ -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<ParmVarDecl>("param");
+  const auto *Function =
+      dyn_cast_or_null<FunctionDecl>(Parameter->getParentFunctionOrMethod());
+
+  if (Function == nullptr || Function->isImplicit())
+    return;
+
+  if (!Function->isCanonicalDecl())
+    return;
+
+  if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(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<QualType>("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

Added: clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h?rev=259530&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h (added)
+++ clang-tools-extra/trunk/clang-tidy/google/NonConstReferences.h Tue Feb  2 11:27:01 2016
@@ -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

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst?rev=259530&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/google-runtime-references.rst Tue Feb  2 11:27:01 2016
@@ -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

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=259530&r1=259529&r2=259530&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Feb  2 11:27:01 2016
@@ -38,6 +38,7 @@ Clang-Tidy Checks
    google-runtime-member-string-references
    google-runtime-memset
    google-runtime-operator
+   google-runtime-references
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment

Added: clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp?rev=259530&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-references.cpp Tue Feb  2 11:27:01 2016
@@ -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<typename T>
+void g7(const T& t) {}
+
+template<typename T>
+void g8(T t) {}
+
+void f5() {
+  g5(f5);
+  g6(f5);
+  g7(f5);
+  g7<F&>(f5);
+  g8(f5);
+  g8<F&>(f5);
+}
+
+// Don't warn on dependent types.
+template<typename T>
+void g9(T& t) {}
+template<typename T>
+void g10(T t) {}
+
+void f6() {
+  int i;
+  float f;
+  g9<int>(i);
+  g9<const int>(i);
+  g9<int&>(i);
+  g10<int&>(i);
+  g10<float&>(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; }




More information about the cfe-commits mailing list