hokein updated this revision to Diff 43826.
hokein added a comment.

Move unittest to lit test.


http://reviews.llvm.org/D15710

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/check_clang_tidy.py
  test/clang-tidy/misc-definitions-in-headers.hpp
  test/lit.cfg

Index: test/lit.cfg
===================================================================
--- test/lit.cfg
+++ test/lit.cfg
@@ -43,7 +43,8 @@
 config.test_format = lit.formats.ShTest(execute_external)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.modularize', '.module-map-checker']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+  '.modularize', '.module-map-checker', '.hpp']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
 # from test suites; there won't be any lit tests within them.
Index: test/clang-tidy/misc-definitions-in-headers.hpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -0,0 +1,131 @@
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+
+int f() {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline int f() {
+  return 1;
+}
+
+class CA {
+  void f1() {} // ok
+  void f2();
+  template<typename T>
+  T f3() {  // ok
+    T a = 1;
+    return a;
+  }
+  template<typename T>
+  struct CAA {
+    struct CAB {
+      void f4(); // ok
+    };
+  };
+  static void f4(); // ok
+};
+
+void CA::f2() { }
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline void CA::f2() {
+
+template <>
+int CA::f3() {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+  int a = 1;
+  return a;
+}
+
+template <typename T>
+void CA::CAA<T>::CAB::f4() { // ok
+}
+
+template <typename T>
+struct CB {
+  void f1();
+  struct CCA {
+    void f2(T a);
+  };
+  struct CCB;  // ok
+  static int a; // ok
+};
+
+template <typename T>
+void CB<T>::f1() { // ok
+}
+
+template <typename T>
+void CB<T>::CCA::f2(T a) { // ok
+}
+
+template <typename T>
+struct CB<T>::CCB {
+  void f3();
+};
+
+template <typename T>
+void CB<T>::CCB::f3() { // ok
+}
+
+template <typename T>
+int CB<T>::a = 2; // ok;
+
+template <typename T>
+T tf() { // ok
+  T a;
+  return a;
+}
+
+
+namespace NA {
+  int f() { return 1; }
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+// CHECK-FIXES: inline int f() { return 1; }
+}
+
+template <typename T>
+T f3() {
+  T a = 1;
+  return a;
+}
+
+template <>
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: function definition is not allowed in header file [misc-definitions-in-headers]
+int f3() {
+  int a = 1;
+  return a;
+}
+
+int f5(); // ok
+inline int f6() { return 1; } // ok
+namespace {
+  int f7() { return 1; } // ok
+}
+
+int a = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+CA a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+namespace NB {
+  int b = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+  const int c = 1; // ok;
+}
+
+class CC {
+  static int d;
+};
+
+int CC::d = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+const char* ca = "foo";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable definition is not allowed in header file [misc-definitions-in-headers]
+
+namespace {
+  int e = 2; // ok
+}
+
+const char* const g = "foo"; // ok
+static int h = 1; // ok
+const int i = 1; // ok
+extern int j; // ok
Index: test/clang-tidy/check_clang_tidy.py
===================================================================
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -52,6 +52,8 @@
   extension = '.cpp'
   if (input_file_name.endswith('.c')):
     extension = '.c'
+  if (input_file_name.endswith('.hpp')):
+    extension = '.hpp'
   temp_file_name = temp_file_name + extension
 
   clang_tidy_extra_args = extra_args
Index: docs/clang-tidy/checks/misc-definitions-in-headers.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-definitions-in-headers.rst
@@ -0,0 +1,36 @@
+misc-definitions-in-headers
+===========================
+
+Finds non-extern non-inline function and variable definitions in header files, which can lead to potential ODR violations.
+
+.. code:: c++
+   // Foo.h
+   int a = 1; // error
+   static int b = 1; // ok
+   const int c = 1; // ok
+   extern int d; // ok
+
+   namespace N {
+     int e = 2; // error
+   }
+   namespace {
+     int f = 2; // ok
+   }
+
+   // error
+   int g() {
+     return 1;
+   }
+
+   // ok
+   inline int e() {
+     return 1;
+   }
+
+   class A {
+    public:
+     int f1() { return 1; } // ok
+     int f2();
+   };
+
+   int A::f2() { return 1; } // error
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -37,6 +37,7 @@
    misc-assert-side-effect
    misc-assign-operator-signature
    misc-bool-pointer-implicit-conversion
+   misc-definitions-in-headers
    misc-inaccurate-erase
    misc-inefficient-algorithm
    misc-macro-parentheses
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "AssertSideEffectCheck.h"
 #include "AssignOperatorSignatureCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
+#include "DefinitionsInHeadersCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "InefficientAlgorithmCheck.h"
 #include "MacroParenthesesCheck.h"
@@ -47,6 +48,8 @@
         "misc-assign-operator-signature");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "misc-bool-pointer-implicit-conversion");
+    CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
+        "misc-definitions-in-headers");
     CheckFactories.registerCheck<InaccurateEraseCheck>(
         "misc-inaccurate-erase");
     CheckFactories.registerCheck<InefficientAlgorithmCheck>(
Index: clang-tidy/misc/DefinitionsInHeadersCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/DefinitionsInHeadersCheck.h
@@ -0,0 +1,35 @@
+//===--- DefinitionsInHeadersCheck.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_MISC_DEFINITIONS_IN_HEADERS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+// Finds non-extern non-inline function and variable definitions in header
+// files, which can lead to potential ODR violations.
+// For the user-facing documentation see:
+// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html
+class DefinitionsInHeadersCheck : public ClangTidyCheck {
+public:
+  DefinitionsInHeadersCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -0,0 +1,99 @@
+//===--- DefinitionsInHeadersCheck.cpp - clang-tidy------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "DefinitionsInHeadersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void DefinitionsInHeadersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())))
+               .bind("name-decl"),
+      this);
+}
+
+void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
+  // C++ [basic.def.odr] p6:
+  // There can be more than one definition of a class type, enumeration type,
+  // inline function with external linkage, class template, non-static function
+  // template, static data member of a class template, member function of a
+  // class template, or template specialization for which some template
+  // parameters are not specifiedin a program provided that each definition
+  // appears in a different translation unit, and provided the definitions
+  // satisfy the following requirements.
+  const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("name-decl");
+  assert(ND);
+
+  const SourceManager* SM = Result.SourceManager;
+  SourceLocation ExpansionLoc = SM->getExpansionLoc(ND->getLocStart());
+  StringRef Filename = SM->getFilename(ExpansionLoc);
+  bool isHeadeFile = Filename.endswith(".h") || Filename.endswith(".hh") ||
+                     Filename.endswith(".hpp") || Filename.endswith(".hxx");
+  if (!isHeadeFile)
+    return;
+
+  // Internal linkage variable and function definitions are allowed:
+  //   const int a = 1;
+  //   static int b = 1;
+  //   namespace {
+  //     int c = 1;
+  //     int f() { return 1; }
+  //   }
+  if (ND->getLinkageInternal() == InternalLinkage ||
+      ND->getLinkageInternal() == UniqueExternalLinkage)
+    return;
+  if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
+    // Inline function is allowed.
+    if (FD->isInlined())
+      return;
+    // Function template is allowed.
+    if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+      return;
+    // Function template full specialization is prohibited in header file.
+    if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+      return;
+    // Member function of a class template and member function of a nested class
+    // in a class template are allowed.
+    if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+      const auto *DC = MD->getDeclContext();
+      while (DC->isRecord()) {
+        if (const auto *RD = dyn_cast<CXXRecordDecl>(DC))
+          if (RD->getDescribedClassTemplate())
+            return;
+        DC = DC->getParent();
+      }
+    }
+
+    diag(FD->getLocation(), "function definition is not allowed in header file")
+        << FixItHint::CreateInsertion(FD->getSourceRange().getBegin(),
+                                      "inline ");
+  } else if (const auto *VD = dyn_cast<VarDecl>(ND)) {
+    // Static data member of a class template is allowed.
+    if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
+      return;
+    if (VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+      return;
+    // Ignore variable definition within function scope.
+    if (VD->hasLocalStorage() || VD->isStaticLocal())
+      return;
+
+    diag(VD->getLocation(),
+         "variable definition is not allowed in header file");
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -5,6 +5,7 @@
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
+  DefinitionsInHeadersCheck.cpp
   InaccurateEraseCheck.cpp
   InefficientAlgorithmCheck.cpp
   MacroParenthesesCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to