omtcyfz updated this revision to Diff 74509.
omtcyfz marked 5 inline comments as done.
omtcyfz added a comment.
Herald added a subscriber: modocache.

Addressed bunch of comments.


https://reviews.llvm.org/D25024

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.cpp
  clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-one-name-per-declaration.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-one-name-per-declaration.cpp

Index: test/clang-tidy/cppcoreguidelines-one-name-per-declaration.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-one-name-per-declaration.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-one-name-per-declaration %t
+
+int main() {
+  {
+    int x = 42;
+    int y = 43;
+    // No warning here.
+  }
+  {
+    int x = 42, y = 43;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare more than one variable per declaration [cppcoreguidelines-one-name-per-declaration]
+  }
+  {
+    int x = 42, y = 43, z = 44;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare more than one variable per declaration
+  }
+  for (int i = 0, j = 100; i < j; i++);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not declare more than one variable per declaration
+
+  char *p, c, a[7], *pp[7], **aa[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare more than one variable per declaration
+  return 0;
+}
+
+void foo(int x, int y, int z) {} // Expect no warning.
+
+typedef int int_t, *intp_t, (&fp)(int, long), arr_t[10];
+
+template <typename T, typename U>
+void bar() {}
+
+template <class InputIterator, class Predicate>
+bool any_of(InputIterator first, InputIterator last, Predicate pred);
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
    boost-use-to-string
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
+   cert-dcl04-c (redirects to cppcoreguidelines-one-name-per-declaration) <cert-dcl-04-c>
    cert-dcl50-cpp
    cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp>
    cert-dcl59-cpp (redirects to google-build-namespaces) <cert-dcl59-cpp>
@@ -19,6 +20,7 @@
    cert-flp30-c
    cert-oop11-cpp (redirects to misc-move-constructor-init) <cert-oop11-cpp>
    cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-one-name-per-declaration
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index
    cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-one-name-per-declaration.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-one-name-per-declaration.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - cppcoreguidelines-one-name-per-declaration
+
+cppcoreguidelines-one-name-per-declaration
+==========================================
+
+Checks for declarations with multiple names. C++ Core Guidelines suggests to
+split such declarations into multiple declarations each containing one name.
+
+This would improve readability and prevents potential bugs caused by inattention
+and C/C++ syntax specifics.
+
+Example, bad.
+
+.. code-block:: c++
+
+      std::vector<int> velocities(10, 0), numbers(other_numbers),
+                       inputs(input.begin(), input.end());
+
+Example, good.
+
+.. code-block:: c++
+
+      std::vector<int> velocities(10, 0);
+      std::vector<int> numbers(other_numbers);
+      std::vector<int> inputs(input.begin(), input.end());
+
+This rule is part of the "Expressions and statements" profile of the C++ Core
+Guidelines, see
+http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#a-nameres-name-oneaes10-declare-one-name-only-per-declaration
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,9 @@
 Improvements to clang-tidy
 --------------------------
 
+- New `cppcoreguidelines-one-name-per-declaration
+  <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-one-name-per-declaration.html>`_ check
+
 - New `cppcoreguidelines-slicing
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-slicing.html>`_ check
 
Index: clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.h
@@ -0,0 +1,35 @@
+//===--- OneNamePerDeclarationCheck.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_CPPCOREGUIDELINES_ONE_NAME_PER_DECLARATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_ONE_NAME_PER_DECLARATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// This check warns about multiple names in one declaration.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-one-name-per-declaration.html
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+public:
+  OneNamePerDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_ONE_NAME_PER_DECLARATION_H
Index: clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/OneNamePerDeclarationCheck.cpp
@@ -0,0 +1,51 @@
+//===--- OneNamePerDeclarationCheck.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 "OneNamePerDeclarationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+namespace {
+
+AST_MATCHER_P(DeclStmt, declCountIsGreaterThan, unsigned, N) {
+  return std::distance(Node.decl_begin(), Node.decl_end()) > (ptrdiff_t)N;
+}
+
+} // namespace
+
+void OneNamePerDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      declStmt(declCountIsGreaterThan(1)).bind("multipleNameDeclaration"),
+      this);
+}
+
+void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
+  // FIXME: Also provide FixItHint splitting the declarations. E.g.
+  //
+  // Before:
+  //    int x = 42, y = 43, z = 44;
+  // After:
+  //    int x = 42;
+  //    int y = 43;
+  //    int z = 44;
+  const auto *MultipleNameDeclaration =
+      Result.Nodes.getNodeAs<DeclStmt>("multipleNameDeclaration");
+  diag(MultipleNameDeclaration->getStartLoc(),
+       "do not declare more than one variable per declaration");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "InterfacesGlobalInitCheck.h"
+#include "OneNamePerDeclarationCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
@@ -35,6 +36,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
         "cppcoreguidelines-interfaces-global-init");
+    CheckFactories.registerCheck<OneNamePerDeclarationCheck>(
+        "cppcoreguidelines-one-name-per-declaration");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
+  OneNamePerDeclarationCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp
Index: clang-tidy/cert/CMakeLists.txt
===================================================================
--- clang-tidy/cert/CMakeLists.txt
+++ clang-tidy/cert/CMakeLists.txt
@@ -17,6 +17,7 @@
   clangBasic
   clangLex
   clangTidy
+  clangTidyCppCoreGuidelinesModule
   clangTidyGoogleModule
   clangTidyMiscModule
   clangTidyUtils
Index: clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tidy/cert/CERTTidyModule.cpp
@@ -16,6 +16,7 @@
 #include "../misc/NonCopyableObjects.h"
 #include "../misc/StaticAssertCheck.h"
 #include "../misc/ThrowByValueCatchByReferenceCheck.h"
+#include "../cppcoreguidelines/OneNamePerDeclarationCheck.h"
 #include "CommandProcessorCheck.h"
 #include "FloatLoopCounter.h"
 #include "SetLongJmpCheck.h"
@@ -31,7 +32,7 @@
 class CERTModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
-    // C++ checkers
+    // C++ checks
     // DCL
     CheckFactories.registerCheck<VariadicFunctionDefCheck>(
         "cert-dcl50-cpp");
@@ -52,10 +53,12 @@
     CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
         "cert-err61-cpp");
 
-    // C checkers
+    // C checks
     // DCL
     CheckFactories.registerCheck<misc::StaticAssertCheck>(
         "cert-dcl03-c");
+    CheckFactories.registerCheck<cppcoreguidelines::OneNamePerDeclarationCheck>(
+        "cert-dcl04-c");
     // ENV
     CheckFactories.registerCheck<CommandProcessorCheck>(
         "cert-env33-c");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to