[clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

[lebedev.ri]

Summary

The check should ignore the main function, the program entry point.
It is not possible to use std::array<> for the argv.
The alternative is to use char** argv.

Fixes PR40604

[JonasToth]

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

There is FunctionDecl->castToDeclContext() which is probably a better fit here.

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

90 ↗

(On Diff #185400)

main has a third optional argument for environment variables. Please add test case for that, too.
Could add a test for the char **argv-form to ensure these are not ignored somehow?

[lebedev.ri]

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

90 ↗

(On Diff #185400)

char **argv form *is* ignored, that is why i have not seen this issue before.
It's not a C-Style array, but a pointer to a pointer to char.

[lebedev.ri]

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

I'm guessing you meant cast*From*DeclContext().
Interesting, that function is never once used.
And it uses static_cast<>()..

[lebedev.ri]

Address review notes, adjust docs.

[lebedev.ri]

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

I'm not too sure about this.
Given ParmVarDecl, are we sure it's DeclContext is *always* FunctionDecl?

[lebedev.ri]

Add an assert to ensure that clang::FunctionDecl::castFromDeclContext() is safe to do.

[JonasToth]

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

From Doc "Represents a parameter to a function. " so i think it has always a FunctionDecl (or subclass) as DeclContext.

Maybe that function is a relict? I just saw it in the docs too and thought it makes sense to use it. No opinion on that, @aaron.ballman do you know more on that?

[aaron.ballman]

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

From Doc "Represents a parameter to a function. " so i think it has always a FunctionDecl (or subclass) as DeclContext.

ObjCMethodDecl is not a subclass of FunctionDecl, yet it contains ParmVarDecl objects and is a DeclContext.

I would use dyn_cast here instead.

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

58 ↗

(On Diff #185499)

It's -> Its
params -> parameters

[lebedev.ri]

And back to dyn_cast().

[aaron.ballman]

LGTM aside from a nit.

clang-tidy/modernize/AvoidCArraysCheck.cpp

36–38 ↗

(On Diff #185587)

return FD ? FD->isMain() : false; ?

[lebedev.ri]

In D57787#1387406, @aaron.ballman wrote:

LGTM aside from a nit.

Thank you for the review.

clang-tidy/modernize/AvoidCArraysCheck.cpp

35 ↗

(On Diff #185400)

Aha!

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

AST_MATCHER_P(clang::TypeLoc, hasType,
   return TypeNode != nullptr &&
          InnerMatcher.matches(*TypeNode, Finder, Builder);
 }
 
 AST_MATCHER(clang::RecordDecl, isExternCContext) {
   return Node.isExternCContext();
 }
 
+AST_MATCHER(clang::ParmVarDecl, isArgvOfMain) {
+  const clang::DeclContext *DC = Node.getDeclContext();
+  const auto *FD = llvm::dyn_cast<clang::FunctionDecl>(DC);
+  return FD ? FD->isMain() : false;
+}
+
 } // 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())),
+              unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())),
+                           hasParent(varDecl(isExternC())),
                            hasParent(fieldDecl(
                                hasParent(recordDecl(isExternCContext())))),
                            hasAncestor(functionDecl(isExternC())))))
           .bind("typeloc"),
       this);
 }
 
 void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) {

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

.. code:: c++
 
   inline void bar() {
     {
       int j[j[0]]; // not diagnosed
     }
   }
 
   }
+
+Similarly, the ``main()`` function is ignored. Its second and third parameters
+can be either ``char* argv[]`` or ``char** argv``, but can not be
+``std::array<>``.

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

+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+}
+
+int main(int argc, char *argv[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[]) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> instead
+    int f6[] = {1, 2};
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
+  };
+}

clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-c-arrays-ignores-three-arg-main.cpp

+// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t
+
+int not_main(int argc, char *argv[], char *argw[]) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: do not declare C-style arrays, use std::array<> instead
+  int f4[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+}
+
+int main(int argc, char *argv[], char *argw[]) {
+  int f5[] = {1, 2};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std::array<> instead
+
+  auto not_main = [](int argc, char *argv[], char *argw[]) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: do not declare C-style arrays, use std::array<> instead
+    // CHECK-MESSAGES: :[[@LINE-2]]:46: warning: do not declare C-style arrays, use std::array<> instead
+    int f6[] = {1, 2};
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not declare C-style arrays, use std::array<> instead
+  };
+}