[PATCH] Remove 'explicit' keyword from constructors with multiple (non-default) arguments.

[Alexander Kornienko]

Hi djasper,

Remove 'explicit' keyword from constructors with multiple (non-default)
arguments (clang-tidy google-explicit-constructor check).

http://reviews.llvm.org/D4242

Files:
  clang-tidy/google/ExplicitConstructorCheck.cpp
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: clang-tidy/google/ExplicitConstructorCheck.cpp
===================================================================
--- clang-tidy/google/ExplicitConstructorCheck.cpp
+++ clang-tidy/google/ExplicitConstructorCheck.cpp
@@ -55,30 +55,37 @@
   if (Ctor->isOutOfLine() || Ctor->isImplicit() || Ctor->isDeleted())
     return;
 
-  if (Ctor->isExplicit() && Ctor->isCopyOrMoveConstructor()) {
+  bool IsSingleArgumentCtor =
+      Ctor->getNumParams() > 0 && Ctor->getMinRequiredArguments() <= 1;
+  bool RequiredExplicit =
+      !Ctor->isCopyOrMoveConstructor() && IsSingleArgumentCtor;
+
+  if (Ctor->isExplicit() && !RequiredExplicit) {
     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 ConstructorType =
+        !IsSingleArgumentCtor ? "Multiple arguments"
+                              : (Ctor->isMoveConstructor() ? "Move" : "Copy");
     DiagnosticBuilder Diag =
         diag(Ctor->getLocation(), "%0 constructor declared explicit.")
-        << (Ctor->isMoveConstructor() ? "Move" : "Copy");
+        << ConstructorType;
     if (ExplicitTokenRange.isValid()) {
       Diag << FixItHint::CreateRemoval(
           CharSourceRange::getCharRange(ExplicitTokenRange));
     }
-  }
-
-  if (Ctor->isExplicit() || Ctor->isCopyOrMoveConstructor() ||
-      Ctor->getNumParams() == 0 || Ctor->getMinRequiredArguments() > 1)
     return;
+  }
 
-  SourceLocation Loc = Ctor->getLocation();
-  diag(Loc, "Single-argument constructors must be explicit")
-      << FixItHint::CreateInsertion(Loc, "explicit ");
+  if (!Ctor->isExplicit() && RequiredExplicit) {
+    SourceLocation Loc = Ctor->getLocation();
+    diag(Loc, "Single-argument constructors must be explicit")
+        << FixItHint::CreateInsertion(Loc, "explicit ");
+  }
 }
 
 } // namespace tidy
Index: unittests/clang-tidy/GoogleModuleTest.cpp
===================================================================
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -40,11 +40,17 @@
 TEST(ExplicitConstructorCheckTest, RemoveExplicit) {
   EXPECT_EQ("class A { A(const A&); };\n"
             "class B { /*asdf*/  B(B&&); };\n"
-            "class C { /*asdf*/  C(const C&, int i = 0); };",
+            "class C { /*asdf*/  C(const C&, int i = 0); };\n"
+            "class D { D(int a, int b); };\n"
+            "class D2 { explicit D2(int a, int b = 0); };\n"
+            "class E { E(int a, int b, int c = 0); };",
             runCheckOnCode<ExplicitConstructorCheck>(
                 "class A { explicit    A(const A&); };\n"
                 "class B { explicit   /*asdf*/  B(B&&); };\n"
-                "class C { explicit/*asdf*/  C(const C&, int i = 0); };"));
+                "class C { explicit/*asdf*/  C(const C&, int i = 0); };\n"
+                "class D { explicit D(int a, int b); };\n"
+                "class D2 { explicit D2(int a, int b = 0); };\n"
+                "class E { explicit E(int a, int b, int c = 0); };"));
 }
 
 TEST(ExplicitConstructorCheckTest, RemoveExplicitWithMacros) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4242.10723.patch
Type: text/x-patch
Size: 3335 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140621/9849dbe4/attachment.bin>