[clang-tidy] Avoid C arrays check

Summary:
[[ https://bugs.llvm.org/show_bug.cgi?id=39224 | PR39224 ]]
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.

Exceptions: `extern "C"` code.

References:
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack | CPPCG ES.27: Use std::array or stack_array for arrays on the stack ]]
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array | CPPCG SL.con.1: 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`

Reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun

Reviewed By: JonasToth

Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D53771

llvm-svn: 346835

clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

@@ -28,6 +28,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
   clangLex
   clangTidy
   clangTidyMiscModule
+  clangTidyModernizeModule
   clangTidyReadabilityModule
   clangTidyUtils
   clangTooling

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
+#include "../modernize/AvoidCArraysCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
@@ -40,6 +41,8 @@ namespace cppcoreguidelines {
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
+        "cppcoreguidelines-avoid-c-arrays");
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");
     CheckFactories.registerCheck<readability::MagicNumbersCheck>(

clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp

@@ -22,6 +22,7 @@
 #include "../misc/NewDeleteOverloadsCheck.h"
 #include "../misc/StaticAssertCheck.h"
 #include "../bugprone/UndelegatedConstructorCheck.h"
+#include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/DeprecatedHeadersCheck.h"
 #include "../modernize/UseAutoCheck.h"
 #include "../modernize/UseEmplaceCheck.h"
@@ -48,6 +49,8 @@ namespace hicpp {
 class HICPPModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
+        "hicpp-avoid-c-arrays");
     CheckFactories.registerCheck<cppcoreguidelines::AvoidGotoCheck>(
         "hicpp-avoid-goto");
     CheckFactories.registerCheck<readability::BracesAroundStatementsCheck>(

clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp

@@ -0,0 +1,69 @@
+//===--- 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 {
+
+AST_MATCHER(clang::TypeLoc, hasValidBeginLoc) {
+  return Node.getBeginLoc().isValid();
+}
+
+AST_MATCHER_P(clang::TypeLoc, hasType,
+              clang::ast_matchers::internal::Matcher<clang::Type>,
+              InnerMatcher) {
+  const clang::Type *TypeNode = Node.getTypePtr();
+  return TypeNode != nullptr &&
+         InnerMatcher.matches(*TypeNode, Finder, Builder);
+}
+
+AST_MATCHER(clang::RecordDecl, isExternCContext) {
+  return Node.isExternCContext();
+}
+
+} // namespace
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
+  // std::array<> is avaliable since C++11.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(
+      typeLoc(hasValidBeginLoc(), hasType(arrayType()),
+              unless(anyOf(hasParent(varDecl(isExternC())),
+                           hasParent(fieldDecl(
+                               hasParent(recordDecl(isExternCContext())))),
+                           hasAncestor(functionDecl(isExternC())))))
+          .bind("typeloc"),
+      this);
+}
+
+void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *ArrayType = Result.Nodes.getNodeAs<TypeLoc>("typeloc");
+
+  static constexpr llvm::StringLiteral UseArray = llvm::StringLiteral(
+      "do not declare C-style arrays, use std::array<> instead");
+  static constexpr llvm::StringLiteral UseVector = llvm::StringLiteral(
+      "do not declare C VLA arrays, use std::vector<> instead");
+
+  diag(ArrayType->getBeginLoc(),
+       ArrayType->getTypePtr()->isVariableArrayType() ? UseVector : UseArray);
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/modernize/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_MODERNIZE_AVOIDCARRAYSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDCARRAYSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Find C-style array types and recommend to use std::array<> / std::vector<>.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-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 modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDCARRAYSCHECK_H

clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
+  AvoidCArraysCheck.cpp
   ConcatNestedNamespacesCheck.cpp
   DeprecatedHeadersCheck.cpp
   DeprecatedIosBaseAliasesCheck.cpp

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
+#include "AvoidCArraysCheck.h"
 #include "ConcatNestedNamespacesCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "DeprecatedIosBaseAliasesCheck.h"
@@ -48,6 +49,7 @@ class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
+    CheckFactories.registerCheck<AvoidCArraysCheck>("modernize-avoid-c-arrays");
     CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
         "modernize-concat-nested-namespaces");
     CheckFactories.registerCheck<DeprecatedHeadersCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -130,6 +130,12 @@ Improvements to clang-tidy
   but also have logic (non-static member functions), and diagnoses all member
   variables that have any other scope other than ``private``.
 
+- New :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize-avoid-c-arrays>` check.
+
+  Finds C-style array types and recommend to use ``std::array<>`` /
+  ``std::vector<>``.
+
 - New :doc:`modernize-concat-nested-namespaces
   <clang-tidy/checks/modernize-concat-nested-namespaces>` check.
 
@@ -173,12 +179,22 @@ Improvements to clang-tidy
   <clang-tidy/checks/readability-uppercase-literal-suffix>`
   added.
 
+- New alias :doc:`cppcoreguidelines-avoid-c-arrays
+  <clang-tidy/checks/cppcoreguidelines-avoid-c-arrays>`
+  to :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize-avoid-c-arrays>` added.
+
 - New alias :doc:`cppcoreguidelines-non-private-member-variables-in-classes
   <clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes>`
   to :doc:`misc-non-private-member-variables-in-classes
   <clang-tidy/checks/misc-non-private-member-variables-in-classes>`
   added.
 
+- New alias :doc:`hicpp-avoid-c-arrays
+  <clang-tidy/checks/hicpp-avoid-c-arrays>`
+  to :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize-avoid-c-arrays>` added.
+
 - New alias :doc:`hicpp-uppercase-literal-suffix
   <clang-tidy/checks/hicpp-uppercase-literal-suffix>` to
   :doc:`readability-uppercase-literal-suffix

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst

@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-c-arrays
+.. meta::
+   :http-equiv=refresh: 5;URL=modernize-avoid-c-arrays.html
+
+cppcoreguidelines-avoid-c-arrays
+================================
+
+The cppcoreguidelines-avoid-c-arrays check is an alias, please see
+`modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_
+for more information.

clang-tools-extra/docs/clang-tidy/checks/hicpp-avoid-c-arrays.rst

@@ -0,0 +1,10 @@
+.. title:: clang-tidy - hicpp-avoid-c-arrays
+.. meta::
+   :http-equiv=refresh: 5;URL=modernize-avoid-c-arrays.html
+
+hicpp-avoid-c-arrays
+====================
+
+The hicpp-avoid-c-arrays check is an alias, please see
+`modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_
+for more information.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -88,6 +88,7 @@ Clang-Tidy Checks
    cert-msc51-cpp
    cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp>
    cppcoreguidelines-avoid-goto
+   cppcoreguidelines-avoid-c-arrays (redirects to modernize-avoid-c-arrays) <cppcoreguidelines-avoid-c-arrays>
    cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) <cppcoreguidelines-avoid-magic-numbers>
    cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
    cppcoreguidelines-interfaces-global-init
@@ -132,6 +133,7 @@ Clang-Tidy Checks
    google-runtime-int
    google-runtime-operator
    google-runtime-references
+   hicpp-avoid-c-arrays (redirects to modernize-avoid-c-arrays) <hicpp-avoid-c-arrays>
    hicpp-avoid-goto
    hicpp-braces-around-statements (redirects to readability-braces-around-statements) <hicpp-braces-around-statements>
    hicpp-deprecated-headers (redirects to modernize-deprecated-headers) <hicpp-deprecated-headers>
@@ -179,6 +181,7 @@ Clang-Tidy Checks
    misc-unused-parameters
    misc-unused-using-decls
    modernize-avoid-bind
+   modernize-avoid-c-arrays
    modernize-concat-nested-namespaces
    modernize-deprecated-headers
    modernize-deprecated-ios-base-aliases

clang-tools-extra/docs/clang-tidy/checks/modernize-avoid-c-arrays.rst

@@ -0,0 +1,56 @@
+.. title:: clang-tidy - modernize-avoid-c-arrays
+
+modernize-avoid-c-arrays
+========================
+
+`cppcoreguidelines-avoid-c-arrays` redirects here as an alias for this check.
+
+`hicpp-avoid-c-arrays` redirects here as an alias for this check.
+
+Finds C-style array types and recommend to use ``std::array<>`` /
+``std::vector<>``. All types of C arrays are diagnosed.
+
+However, fix-it are potentially dangerous in header files and are therefore not
+emitted right now.
+
+.. 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 VLA arrays, use std::vector<> 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
+  };
+
+  array<int[4], 2> d; // warning: do not declare C-style arrays, use std::array<> instead
+
+  using k = int[4]; // 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
+    }
+  }
+
+  }

clang-tools-extra/test/clang-tidy/modernize-avoid-c-arrays.cpp

@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int a[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+int b[1];
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+void foo() {
+  int c[b[0]];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C VLA arrays, use std::vector<> instead
+
+  using d = decltype(c);
+  d e;
+  // Semi-FIXME: we do not diagnose these last two lines separately,
+  // because we point at typeLoc.getBeginLoc(), which is the decl before that
+  // (int c[b[0]];), which is already diagnosed.
+}
+
+template <typename T, int Size>
+class array {
+  T d[Size];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  int e[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+
+array<int[4], 2> d;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead
+
+using k = int[4];
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays, use std::array<> instead
+
+array<k, 2> dk;
+
+template <typename T>
+class unique_ptr {
+  T *d;
+
+  int e[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+
+unique_ptr<int[]> d2;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+
+using k2 = int[];
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not declare C-style arrays, use std::array<> instead
+
+unique_ptr<k2> dk2;
+
+// Some header
+extern "C" {
+
+int f[] = {1, 2};
+
+int j[1];
+
+inline void bar() {
+  {
+    int j[j[0]];
+  }
+}
+
+extern "C++" {
+int f3[] = {1, 2};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+int j3[1];
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+
+struct Foo {
+  int f3[3] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  int j3[1];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+};
+}
+
+struct Bar {
+
+  int f[3] = {1, 2};
+
+  int j[1];
+};
+}