lebedev.ri created this revision.
lebedev.ri added reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: rnkovacs, mgorny.
lebedev.ri edited the summary of this revision.

PR39224 <https://bugs.llvm.org/show_bug.cgi?id=39224>
As discussed, we can't always do the transform automatically due to that 
array-to-pointer decay of C array.
In order to detect whether we can do said transform, we'd need to be able to 
see all usages of said array,
which is, i would say, rather impossible if e.g. it is in the header.
Thus right now no fixit exists.

Also, is `misc` the best place for this check?

Exceptions: `extern "C"` code.

References:

- CPPCG ES.27: Use std::array or stack_array for arrays on the stack 
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack>
- CPPCG SL.con.1: Prefer using STL array or vector instead of a C array 
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array>
- HICPP `4.1.1 Ensure that a function argument does not undergo an 
array-to-pointer conversion`
- MISRA `5-2-12 An identifier with array type passed as a function argument 
shall not decay to a pointer`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53771

Files:
  clang-tidy/misc/AvoidCArraysCheck.cpp
  clang-tidy/misc/AvoidCArraysCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-avoid-c-arrays.rst
  test/clang-tidy/misc-avoid-c-arrays.cpp

Index: test/clang-tidy/misc-avoid-c-arrays.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-avoid-c-arrays.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s misc-avoid-c-arrays %t
+
+int a[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
+
+int b[1];
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
+
+void foo() {
+  int c[b[0]];
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead
+}
+
+template <typename T, int Size>
+class array {
+  T d[Size];
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
+
+  int e[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead
+};
+
+// Some header
+extern "C" {
+
+int f[] = {1, 2};
+
+int j[1];
+
+inline void bar() {
+  {
+    int j[j[0]];
+  }
+}
+}
Index: docs/clang-tidy/checks/misc-avoid-c-arrays.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-avoid-c-arrays.rst
@@ -0,0 +1,52 @@
+.. title:: clang-tidy - misc-avoid-c-arrays
+
+misc-avoid-c-arrays
+===================
+
+Find C-style array delarations and recommend to use ``std::array<>``.
+All types of C arrays are diagnosed.
+
+Hoever, no fix-its are provided. Such transform would need to be able to
+observe *all* the uses of said declaration in order to decide whether it is
+safe to transform or not, and that is impossible in case of headers.
+
+.. code:: c++
+
+  int a[] = {1, 2}; // warning: do not declare C-style arrays, use std::array<> instead
+
+  int b[1]; // warning: do not declare C-style arrays, use std::array<> instead
+
+  void foo() {
+    int c[b[0]]; // warning: do not declare C-style arrays, use std::array<> instead
+  }
+
+  template <typename T, int Size>
+  class array {
+    T d[Size]; // warning: do not declare C-style arrays, use std::array<> instead
+
+    int e[1]; // warning: do not declare C-style arrays, use std::array<> instead
+  };
+
+  ...
+
+However, the ``extern "C"`` code is ignored, since it is common to share
+such headers between C code, and C++ code.
+
+.. code:: c++
+
+  // Some header
+  extern "C" {
+
+  int f[] = {1, 2}; // not diagnosed
+
+  int j[1]; // not diagnosed
+
+  inline void bar() {
+    {
+      int j[j[0]]; // not diagnosed
+    }
+  }
+
+  }
+
+  ...
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -10,8 +10,8 @@
    abseil-no-internal-dependencies
    abseil-no-namespace
    abseil-redundant-strcat-calls
-   abseil-string-find-startswith
    abseil-str-cat-append
+   abseil-string-find-startswith
    android-cloexec-accept
    android-cloexec-accept4
    android-cloexec-creat
@@ -151,19 +151,20 @@
    hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) <hicpp-special-member-functions>
    hicpp-static-assert (redirects to misc-static-assert) <hicpp-static-assert>
    hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor) <hicpp-undelegated-constructor>
+   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) <hicpp-uppercase-literal-suffix>
    hicpp-use-auto (redirects to modernize-use-auto) <hicpp-use-auto>
    hicpp-use-emplace (redirects to modernize-use-emplace) <hicpp-use-emplace>
    hicpp-use-equals-default (redirects to modernize-use-equals-default) <hicpp-use-equals-default>
    hicpp-use-equals-delete (redirects to modernize-use-equals-delete) <hicpp-use-equals-delete>
    hicpp-use-noexcept (redirects to modernize-use-noexcept) <hicpp-use-noexcept>
    hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
    hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
    hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
-   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) <hicpp-uppercase-literal-suffix>
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment
    llvm-twine-local
+   misc-avoid-c-arrays
    misc-definitions-in-headers
    misc-misplaced-const
    misc-new-delete-overloads
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -116,6 +116,11 @@
   Finds macro usage that is considered problematic because better language
   constructs exist for the task.
 
+- New :doc:`misc-avoid-c-arrays
+  <clang-tidy/checks/misc-avoid-c-arrays>` check.
+
+  Finds C-style array delarations and recommend to use ``std::array<>``.
+
 - New :doc:`misc-non-private-member-variables-in-classes
   <clang-tidy/checks/misc-non-private-member-variables-in-classes>` check.
 
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidCArraysCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
@@ -31,6 +32,7 @@
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<AvoidCArraysCheck>("misc-avoid-c-arrays");
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
         "misc-definitions-in-headers");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
+  AvoidCArraysCheck.cpp
   DefinitionsInHeadersCheck.cpp
   MiscTidyModule.cpp
   MisplacedConstCheck.cpp
Index: clang-tidy/misc/AvoidCArraysCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/AvoidCArraysCheck.h
@@ -0,0 +1,35 @@
+//===--- AvoidCArraysCheck.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_AVOIDCARRAYSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_AVOIDCARRAYSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find C-style array delarations and recommend to use std::array<>.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-avoid-c-arrays.html
+class AvoidCArraysCheck : public ClangTidyCheck {
+public:
+  AvoidCArraysCheck(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_AVOIDCARRAYSCHECK_H
Index: clang-tidy/misc/AvoidCArraysCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/AvoidCArraysCheck.cpp
@@ -0,0 +1,41 @@
+//===--- AvoidCArraysCheck.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 "AvoidCArraysCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
+  // std::array<> is avaliable since C++11.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(
+      valueDecl(allOf(hasType(arrayType()),
+                      unless(anyOf(varDecl(isExternC()),
+                                   hasAncestor(functionDecl(isExternC()))))))
+          .bind("decl"),
+      this);
+}
+
+void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *ArrayDecl = Result.Nodes.getNodeAs<Decl>("decl");
+  diag(ArrayDecl->getLocation(),
+       "do not declare C-style arrays, use std::array<> instead");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to