From 15c5e6a5975966255aeefdd7aab7006edb497fcd Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Thu, 27 Nov 2014 11:11:47 +0000 Subject: [PATCH] [clang-tidy] Support initializer_list in google-explicit-constructor check 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. Reviewers: djasper Reviewed By: djasper Subscribers: curdeius, cfe-commits Differential Revision: http://reviews.llvm.org/D6427 llvm-svn: 222878 --- .../clang-tidy/google/ExplicitConstructorCheck.cpp | 40 ++++++++++++--- .../test/clang-tidy/deduplication.cpp | 2 +- clang-tools-extra/test/clang-tidy/diagnostic.cpp | 4 +- clang-tools-extra/test/clang-tidy/file-filter.cpp | 20 ++++---- .../clang-tidy/google-explicit-constructor.cpp | 59 ++++++++++++++++++++++ clang-tools-extra/test/clang-tidy/line-filter.cpp | 14 ++--- clang-tools-extra/test/clang-tidy/macros.cpp | 2 +- clang-tools-extra/test/clang-tidy/nolint.cpp | 6 +-- 8 files changed, 117 insertions(+), 30 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/google-explicit-constructor.cpp diff --git a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp index 764f180..a07902e 100644 --- a/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp @@ -47,15 +47,33 @@ SourceRange FindToken(const SourceManager &Sources, LangOptions LangOpts, return SourceRange(); } +bool isStdInitializerList(QualType Type) { + if (const RecordType *RT = Type.getCanonicalType()->getAs()) { + if (ClassTemplateSpecializationDecl *Specialization = + dyn_cast(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("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"; @@ -63,21 +81,31 @@ void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) { 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 should not be 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 "); } diff --git a/clang-tools-extra/test/clang-tidy/deduplication.cpp b/clang-tools-extra/test/clang-tidy/deduplication.cpp index 36334ca..8a912fa 100644 --- a/clang-tools-extra/test/clang-tidy/deduplication.cpp +++ b/clang-tools-extra/test/clang-tidy/deduplication.cpp @@ -2,7 +2,7 @@ template 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: diff --git a/clang-tools-extra/test/clang-tidy/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/diagnostic.cpp index b223fb7..92ee515 100644 --- a/clang-tools-extra/test/clang-tidy/diagnostic.cpp +++ b/clang-tools-extra/test/clang-tidy/diagnostic.cpp @@ -14,8 +14,8 @@ // 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: diff --git a/clang-tools-extra/test/clang-tidy/file-filter.cpp b/clang-tools-extra/test/clang-tidy/file-filter.cpp index c986dec..8a9d031 100644 --- a/clang-tools-extra/test/clang-tidy/file-filter.cpp +++ b/clang-tools-extra/test/clang-tidy/file-filter.cpp @@ -5,27 +5,27 @@ #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 // 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: diff --git a/clang-tools-extra/test/clang-tidy/google-explicit-constructor.cpp b/clang-tools-extra/test/clang-tidy/google-explicit-constructor.cpp new file mode 100644 index 0000000..7866df6 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/google-explicit-constructor.cpp @@ -0,0 +1,59 @@ +// 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 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 list1) {} + + explicit A(void *x) {} + explicit A(void *x, void *y) {} + + explicit A(const A& a) {} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: copy constructor should not be declared explicit [google-explicit-constructor] + // CHECK-FIXES: {{^ }}A(const A& a) {} + + explicit A(std::initializer_list list2) {} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: initializer-list constructor should not be declared explicit [google-explicit-constructor] + // CHECK-FIXES: {{^ }}A(std::initializer_list 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) {} +}; diff --git a/clang-tools-extra/test/clang-tidy/line-filter.cpp b/clang-tools-extra/test/clang-tidy/line-filter.cpp index a2fdbb4..a6a1885 100644 --- a/clang-tools-extra/test/clang-tidy/line-filter.cpp +++ b/clang-tools-extra/test/clang-tidy/line-filter.cpp @@ -2,25 +2,25 @@ #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: diff --git a/clang-tools-extra/test/clang-tidy/macros.cpp b/clang-tools-extra/test/clang-tidy/macros.cpp index dc5d131..91e38ac 100644 --- a/clang-tools-extra/test/clang-tidy/macros.cpp +++ b/clang-tools-extra/test/clang-tidy/macros.cpp @@ -3,5 +3,5 @@ #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' diff --git a/clang-tools-extra/test/clang-tidy/nolint.cpp b/clang-tools-extra/test/clang-tidy/nolint.cpp index 5b4b454..05d05b8 100644 --- a/clang-tools-extra/test/clang-tidy/nolint.cpp +++ b/clang-tools-extra/test/clang-tidy/nolint.cpp @@ -1,11 +1,11 @@ // 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) -- 2.7.4