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

[David Blaikie]

Fwiw, if I'm not mistaken, explicit on more - than - one - arg actors does
have some meaning in c++11. It means you can't construct from a braced init
list.

I'm not sure if that's important to you/this warning.
On Jun 21, 2014 4:26 PM, "Alexander Kornienko" <alexfh at google.com> wrote:

> 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) {
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140621/35b302e0/attachment.html>