Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Add AllowStringArrays option to modernize-avoid-c-arrays #71701

Conversation

PiotrZSL
Copy link
Member

@PiotrZSL PiotrZSL commented Nov 8, 2023

Add AllowStringArrays option, enabling the exclusion of array types with deduced sizes constructed from string literals. This includes only var declarations of array of characters constructed directly from c-strings.

Closes #59475

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 8, 2023

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Add AllowStringArrays option, enabling the exclusion of array types with deduced sizes constructed from string literals. This includes only var declarations of array of characters constructed directly from c-strings.

Closes #59475


Full diff: https://github.com/llvm/llvm-project/pull/71701.diff

6 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp (+19-2)
  • (modified) clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h (+10-4)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst (+9)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp (+6)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp (+3)
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
index d1b15479ffe7a93..ab1cdd62aa2cc01 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
@@ -12,6 +12,8 @@
 
 using namespace clang::ast_matchers;
 
+namespace clang::tidy::modernize {
+
 namespace {
 
 AST_MATCHER(clang::TypeLoc, hasValidBeginLoc) {
@@ -38,16 +40,31 @@ AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
 
 } // namespace
 
-namespace clang::tidy::modernize {
+AvoidCArraysCheck::AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      AllowStringArrays(Options.get("AllowStringArrays", false)) {}
+
+void AvoidCArraysCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowStringArrays", AllowStringArrays);
+}
 
 void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) {
+  ast_matchers::internal::Matcher<TypeLoc> IgnoreStringArrayIfNeededMatcher =
+      anything();
+  if (AllowStringArrays)
+    IgnoreStringArrayIfNeededMatcher =
+        unless(typeLoc(loc(hasCanonicalType(incompleteArrayType(
+                           hasElementType(isAnyCharacter())))),
+                       hasParent(varDecl(hasInitializer(stringLiteral())))));
+
   Finder->addMatcher(
       typeLoc(hasValidBeginLoc(), hasType(arrayType()),
               unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
                            hasParent(varDecl(isExternC())),
                            hasParent(fieldDecl(
                                hasParent(recordDecl(isExternCContext())))),
-                           hasAncestor(functionDecl(isExternC())))))
+                           hasAncestor(functionDecl(isExternC())))),
+              std::move(IgnoreStringArrayIfNeededMatcher))
           .bind("typeloc"),
       this);
 }
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h
index 7099f99c869498f..719e88e4b31662f 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.h
@@ -19,13 +19,19 @@ namespace clang::tidy::modernize {
 /// 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) {}
+  AvoidCArraysCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus11;
   }
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  const bool AllowStringArrays;
 };
 
 } // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fe8c7175d554c7b..97e532abbf8c715 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -330,6 +330,11 @@ Changes in existing checks
   <clang-tidy/checks/modernize/avoid-bind>` check to
   not emit a ``return`` for fixes when the function returns ``void``.
 
+- Improved :doc:`modernize-avoid-c-arrays
+  <clang-tidy/checks/modernize/avoid-c-arrays>` check by introducing the new
+  `AllowStringArrays` option, enabling the exclusion of array types with deduced
+  sizes constructed from string literals.
+
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst
index bc61033ff1fa1fa..acc628d58f80ee0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst
@@ -58,3 +58,12 @@ such headers between C code, and C++ code.
 Similarly, the ``main()`` function is ignored. Its second and third parameters
 can be either ``char* argv[]`` or ``char** argv``, but cannot be
 ``std::array<>``.
+
+.. option:: AllowStringArrays
+
+  When set to `true` (default is `false`), incomplete array types constructed
+  from string literals will be ignored. Example:
+
+  .. code:: c++
+
+    const char name[] = "Some name";
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp
new file mode 100644
index 000000000000000..545429b2be807e9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays-ignores-strings.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t -- \
+// RUN:  -config='{CheckOptions: { modernize-avoid-c-arrays.AllowStringArrays: true }}'
+
+const char name[] = "name";
+const char array[] = {'n', 'a', 'm', 'e', '\0'};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
index dd3078010eb3805..4233f69337898ee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp
@@ -86,3 +86,6 @@ struct Bar {
   int j[1];
 };
 }
+
+const char name[] = "Some string";
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]

Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'd prefer a second person to approve as well.

Copy link
Contributor

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may not work for

void f(const char name[]);

f("111");

Add AllowStringArrays option, enabling the exclusion of array types with deduced
sizes constructed from string literals. This includes only var declarations
of array of characters constructed directly from c-strings.
@PiotrZSL PiotrZSL force-pushed the 59475-clang-tidy-modernize-avoid-c-arrays-option-to-ignore-c-style-strings branch from 8301bee to b4c4573 Compare January 29, 2024 22:44
@PiotrZSL
Copy link
Member Author

I may not work for

void f(const char name[]);

f("111");

It should not work, as this is not array, but pointer. If you do sizeof(name) you get size of pointer, instead of number of elements in array.

Copy link

github-actions bot commented Jan 29, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@HerrCai0907
Copy link
Contributor

It should not work, as this is not array, but pointer

Agree. But it fulfills the description of this opinion, since it looks like to construct a array from string literals.
It would be better to explain it in document.

incomplete array types constructed from string literals will be ignored.

@PiotrZSL PiotrZSL merged commit b777bb7 into llvm:main Feb 1, 2024
5 checks passed
@PiotrZSL PiotrZSL deleted the 59475-clang-tidy-modernize-avoid-c-arrays-option-to-ignore-c-style-strings branch February 1, 2024 06:05
carlosgalvezp pushed a commit to carlosgalvezp/llvm-project that referenced this pull request Feb 1, 2024
…llvm#71701)

Add AllowStringArrays option, enabling the exclusion of array types with
deduced sizes constructed from string literals. This includes only var
declarations of array of characters constructed directly from c-strings.

Closes llvm#59475
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
…llvm#71701)

Add AllowStringArrays option, enabling the exclusion of array types with
deduced sizes constructed from string literals. This includes only var
declarations of array of characters constructed directly from c-strings.

Closes llvm#59475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] modernize-avoid-c-arrays option to ignore C-style strings
4 participants