[clang-tidy] Support initializer_list in google-explicit-constructor check

[alexfh]

Summary

According to the Google C++ Style Guide, constructors taking a single
std::initializer_list<> should not be marked explicit.

This change also changes the messages according to conventions used in Clang
diagnostics: no capitalization of the first letter, no trailing dot.

[djasper]

Looks good.

clang-tidy/google/ExplicitConstructorCheck.cpp

93

Maybe make this call for action more, e.g. "%0 constructor must not be declared explicit".

[alexfh]

clang-tidy/google/ExplicitConstructorCheck.cpp

93

Changed the message to "X should not be declared explicit" to match the Style Guide wording.

clang-tidy/google/ExplicitConstructorCheck.cpp

if (Pred(Tok)) {
       Lex.LexFromRawLexer(NextTok);
       return SourceRange(Tok.getLocation(), NextTok.getLocation());
     }
   } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc);
 
   return SourceRange();
 }
 
+bool isStdInitializerList(QualType Type) {
+  if (const RecordType *RT = Type.getCanonicalType()->getAs<RecordType>()) {
+    if (ClassTemplateSpecializationDecl *Specialization =
+            dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl())) {
+      ClassTemplateDecl *Template = Specialization->getSpecializedTemplate();
+      // First use the fast getName() method to avoid unnecessary calls to the
+      // slow getQualifiedNameAsString().
+      return Template->getName() == "initializer_list" &&
+             Template->getQualifiedNameAsString() == "std::initializer_list";
+    }
+  }
+  return false;
+}
+
 void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
   const CXXConstructorDecl *Ctor =
       Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
   // Do not be confused: isExplicit means 'explicit' keyword is present,
   // isImplicit means that it's a compiler-generated constructor.
-  if (Ctor->isOutOfLine() || Ctor->isImplicit() || Ctor->isDeleted())
+  if (Ctor->isOutOfLine() || Ctor->isImplicit() || Ctor->isDeleted() ||
+      Ctor->getNumParams() == 0 || Ctor->getMinRequiredArguments() > 1)
     return;
 
-  if (Ctor->isExplicit() && Ctor->isCopyOrMoveConstructor()) {
+  bool takesInitializerList = isStdInitializerList(
+      Ctor->getParamDecl(0)->getType().getNonReferenceType());
+  if (Ctor->isExplicit() &&
+      (Ctor->isCopyOrMoveConstructor() || takesInitializerList)) {
     auto isKWExplicit = [](const Token &Tok) {
       return Tok.is(tok::raw_identifier) &&
              Tok.getRawIdentifier() == "explicit";
     };
     SourceRange ExplicitTokenRange =
         FindToken(*Result.SourceManager, Result.Context->getLangOpts(),
                   Ctor->getOuterLocStart(), Ctor->getLocEnd(), isKWExplicit);
+    StringRef ConstructorDescription;
+    if (Ctor->isMoveConstructor())
+      ConstructorDescription = "move";
+    else if (Ctor->isCopyConstructor())
+      ConstructorDescription = "copy";
+    else
+      ConstructorDescription = "initializer-list";
+
     DiagnosticBuilder Diag =
-        diag(Ctor->getLocation(), "%0 constructor declared explicit.")
-        << (Ctor->isMoveConstructor() ? "Move" : "Copy");
+        diag(Ctor->getLocation(), "%0 constructor declared explicit")
+        << ConstructorDescription;
     if (ExplicitTokenRange.isValid()) {
       Diag << FixItHint::CreateRemoval(
           CharSourceRange::getCharRange(ExplicitTokenRange));
     }
+    return;
   }
 
   if (Ctor->isExplicit() || Ctor->isCopyOrMoveConstructor() ||
-      Ctor->getNumParams() == 0 || Ctor->getMinRequiredArguments() > 1)
+      takesInitializerList)
     return;
 
   SourceLocation Loc = Ctor->getLocation();
-  diag(Loc, "Single-argument constructors must be explicit")
+  diag(Loc, "single-argument constructors must be explicit")
       << FixItHint::CreateInsertion(Loc, "explicit ");
 }
 
 } // namespace tidy
 } // namespace clang

test/clang-tidy/deduplication.cpp

 // RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s
 
 template<typename T>
 struct A { A(T); };
-// CHECK: :[[@LINE-1]]:12: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK: :[[@LINE-1]]:12: warning: single-argument constructors must be explicit [google-explicit-constructor]
 // CHECK-NOT: warning:
 
 
 void f() {
   A<int> a(0);
   A<double> b(0);
 }

test/clang-tidy/diagnostic.cpp

 // RUN: clang-tidy -checks='-*,misc-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s
 // RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s
 // RUN: clang-tidy -checks='-*,misc-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 %s
 
 // CHECK1-NOT: warning
 // CHECK2-NOT: warning
 // CHECK3-NOT: warning
 
 // CHECK1: error: error reading '{{.*}}.nonexistent.cpp' [clang-diagnostic-error]
 // CHECK2: error: unknown argument: '-fan-unknown-option' [clang-diagnostic-error]
 
 // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK3: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' changes value
 int a = 1.5;
 
-// CHECK2: :[[@LINE+2]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
-// CHECK3: :[[@LINE+1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK2: :[[@LINE+2]]:11: warning: single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK3: :[[@LINE+1]]:11: warning: single-argument constructors must be explicit [google-explicit-constructor]
 class A { A(int) {} };
 
 // CHECK2-NOT: warning:
 // CHECK3-NOT: warning:
 
 #define MACRO_FROM_COMMAND_LINE
 // CHECK4: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined

test/clang-tidy/file-filter.cpp

 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK2 %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header2\.h' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK3 %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4 %s
 
 #include "header1.h"
 // CHECK-NOT: warning:
-// CHECK2: header1.h:1:12: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK2: header1.h:1:12: warning: single-argument constructors must be explicit [google-explicit-constructor]
 // CHECK3-NOT: warning:
-// CHECK4: header1.h:1:12: warning: Single-argument constructors
+// CHECK4: header1.h:1:12: warning: single-argument constructors
 
 #include "header2.h"
 // CHECK-NOT: warning:
-// CHECK2: header2.h:1:12: warning: Single-argument constructors
-// CHECK3: header2.h:1:12: warning: Single-argument constructors
-// CHECK4: header2.h:1:12: warning: Single-argument constructors
+// CHECK2: header2.h:1:12: warning: single-argument constructors
+// CHECK3: header2.h:1:12: warning: single-argument constructors
+// CHECK4: header2.h:1:12: warning: single-argument constructors
 
 #include <system-header.h>
 // CHECK-NOT: warning:
 // CHECK2-NOT: warning:
 // CHECK3-NOT: warning:
-// CHECK4: system-header.h:1:12: warning: Single-argument constructors
+// CHECK4: system-header.h:1:12: warning: single-argument constructors
 
 class A { A(int); };
-// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors
-// CHECK2: :[[@LINE-2]]:11: warning: Single-argument constructors
-// CHECK3: :[[@LINE-3]]:11: warning: Single-argument constructors
-// CHECK4: :[[@LINE-4]]:11: warning: Single-argument constructors
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors
+// CHECK2: :[[@LINE-2]]:11: warning: single-argument constructors
+// CHECK3: :[[@LINE-3]]:11: warning: single-argument constructors
+// CHECK4: :[[@LINE-4]]:11: warning: single-argument constructors
 
 // CHECK-NOT: warning:
 // CHECK2-NOT: warning:
 // CHECK3-NOT: warning:
 // CHECK4-NOT: warning:
 
 // CHECK: Suppressed 3 warnings (3 in non-user code)
 // CHECK: Use -header-filter='.*' to display errors from all non-system headers.

test/clang-tidy/google-explicit-constructor.cpp

+// RUN: $(dirname %s)/check_clang_tidy.sh %s google-explicit-constructor %t
+// REQUIRES: shell
+
+namespace std {
+  typedef decltype(sizeof(int)) size_t;
+
+  // libc++'s implementation
+  template <class _E>
+  class initializer_list
+  {
+    const _E* __begin_;
+    size_t    __size_;
+
+    initializer_list(const _E* __b, size_t __s)
+      : __begin_(__b),
+        __size_(__s)
+    {}
+
+  public:
+    typedef _E        value_type;
+    typedef const _E& reference;
+    typedef const _E& const_reference;
+    typedef size_t    size_type;
+
+    typedef const _E* iterator;
+    typedef const _E* const_iterator;
+
+    initializer_list() : __begin_(nullptr), __size_(0) {}
+
+    size_t    size()  const {return __size_;}
+    const _E* begin() const {return __begin_;}
+    const _E* end()   const {return __begin_ + __size_;}
+  };
+}
+
+struct A {
+  A() {}
+  A(int x, int y) {}
+  A(std::initializer_list<int> list1) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  explicit A(const A& a) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor declared explicit [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}A(const A& a) {}
+
+  explicit A(std::initializer_list<double> list2) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: initializer-list constructor declared explicit [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}A(std::initializer_list<double> list2) {}
+
+  A(int x1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: single-argument constructors must be explicit [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit A(int x1) {}
+
+  A(double x2, double y = 3.14) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: single-argument constructors must be explicit
+  // CHECK-FIXES: {{^  }}explicit A(double x2, double y = 3.14) {}
+};

test/clang-tidy/line-filter.cpp

 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -line-filter='[{"name":"line-filter.cpp","lines":[[18,18],[22,22]]},{"name":"header1.h","lines":[[1,2]]},{"name":"header2.h"},{"name":"header3.h"}]' -header-filter='header[12]\.h$' %s -- -I %S/Inputs/line-filter 2>&1 | FileCheck %s
 
 #include "header1.h"
 // CHECK-NOT: header1.h:{{.*}} warning
-// CHECK: header1.h:1:12: warning: Single-argument constructors must be explicit [google-explicit-constructor]
-// CHECK: header1.h:2:12: warning: Single-argument constructors {{.*}}
+// CHECK: header1.h:1:12: warning: single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK: header1.h:2:12: warning: single-argument constructors {{.*}}
 // CHECK-NOT: header1.h:{{.*}} warning
 
 #include "header2.h"
-// CHECK: header2.h:1:12: warning: Single-argument constructors {{.*}}
-// CHECK: header2.h:2:12: warning: Single-argument constructors {{.*}}
-// CHECK: header2.h:3:12: warning: Single-argument constructors {{.*}}
+// CHECK: header2.h:1:12: warning: single-argument constructors {{.*}}
+// CHECK: header2.h:2:12: warning: single-argument constructors {{.*}}
+// CHECK: header2.h:3:12: warning: single-argument constructors {{.*}}
 // CHECK-NOT: header2.h:{{.*}} warning
 
 #include "header3.h"
 // CHECK-NOT: header3.h:{{.*}} warning
 
 class A { A(int); };
-// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors {{.*}}
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors {{.*}}
 class B { B(int); };
 // CHECK-NOT: :[[@LINE-1]]:{{.*}} warning
 class C { C(int); };
-// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors {{.*}}
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors {{.*}}
 
 // CHECK-NOT: warning:
 
 // CHECK: Suppressed 3 warnings (1 in non-user code, 2 due to line filter)

test/clang-tidy/macros.cpp

 // RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s
 
 #define Q(name) class name { name(int i); }
 
 Q(A);
-// CHECK: :[[@LINE-1]]:3: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK: :[[@LINE-1]]:3: warning: single-argument constructors must be explicit [google-explicit-constructor]
 // CHECK: :3:30: note: expanded from macro 'Q'

test/clang-tidy/nolint.cpp

 // RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- 2>&1 | FileCheck %s
 
 class A { A(int i); };
-// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be explicit [google-explicit-constructor]
 
 class B { B(int i); }; // NOLINT
-// CHECK-NOT: :[[@LINE-1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK-NOT: :[[@LINE-1]]:11: warning: single-argument constructors must be explicit [google-explicit-constructor]
 
 class C { C(int i); }; // NOLINT(we-dont-care-about-categories-yet)
-// CHECK-NOT: :[[@LINE-1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
+// CHECK-NOT: :[[@LINE-1]]:11: warning: single-argument constructors must be explicit [google-explicit-constructor]
 // CHECK: Suppressed 2 warnings (2 NOLINT)

test/clang-tidy/serialize-diagnostics.cpp

-// RUN: clang-tidy %s -- --serialize-diagnostics %t | FileCheck %s
+// RUN: clang-tidy %s -- -serialize-diagnostics %t | FileCheck %s
+// RUN: clang-tidy %s -- -diagnostic-log-file %t | FileCheck %s
 // CHECK: :[[@LINE+1]]:12: error: expected ';' after struct [clang-diagnostic-error]
 struct A {}