Index: clang-tidy/google/ExplicitConstructorCheck.cpp =================================================================== --- clang-tidy/google/ExplicitConstructorCheck.cpp +++ clang-tidy/google/ExplicitConstructorCheck.cpp @@ -55,7 +55,12 @@ 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"; @@ -63,22 +68,24 @@ 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( "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) {