[clang-tools-extra] r201308 - Fix ExplicitConstructorCheck to warn only on in-class declarations.

[Alexander Kornienko]

Author: alexfh
Date: Thu Feb 13 04:11:48 2014
New Revision: 201308

URL: http://llvm.org/viewvc/llvm-project?rev=201308&view=rev
Log:
Fix ExplicitConstructorCheck to warn only on in-class declarations.

Summary:
I'm not absolutely sure this is 100% correct solution, but it seems to
do what I expect.

Reviewers: djasper, klimek

Reviewed By: djasper

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D2756

Modified:
    clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
    clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=201308&r1=201307&r2=201308&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Thu Feb 13 04:11:48 2014
@@ -24,20 +24,22 @@ using namespace clang::ast_matchers;
 namespace clang {
 namespace tidy {
 
-void
-ExplicitConstructorCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  Finder->addMatcher(constructorDecl().bind("construct"), this);
+void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(constructorDecl().bind("ctor"), this);
 }
 
 void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
   const CXXConstructorDecl *Ctor =
-      Result.Nodes.getNodeAs<CXXConstructorDecl>("construct");
-  if (!Ctor->isExplicit() && !Ctor->isImplicit() && Ctor->getNumParams() >= 1 &&
-      Ctor->getMinRequiredArguments() <= 1) {
-    SourceLocation Loc = Ctor->getLocation();
-    diag(Loc, "Single-argument constructors must be explicit")
-        << FixItHint::CreateInsertion(Loc, "explicit ");
-  }
+      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->isExplicit() || Ctor->isImplicit())
+    return;
+  if (Ctor->getNumParams() == 0 || Ctor->getMinRequiredArguments() > 1)
+    return;
+  SourceLocation Loc = Ctor->getLocation();
+  diag(Loc, "Single-argument constructors must be explicit")
+      << FixItHint::CreateInsertion(Loc, "explicit ");
 }
 
 class GoogleModule : public ClangTidyModule {

Modified: clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp?rev=201308&r1=201307&r2=201308&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp Thu Feb 13 04:11:48 2014
@@ -21,5 +21,10 @@ TEST_F(ExplicitConstructorCheckTest, Def
             runCheckOn("class C { C(int i, int j = 0); };"));
 }
 
+TEST_F(ExplicitConstructorCheckTest, OutOfLineDefinitions) {
+  EXPECT_EQ("class C { explicit C(int i); }; C::C(int i) {}",
+            runCheckOn("class C { C(int i); }; C::C(int i) {}"));
+}
+
 } // namespace tidy
 } // namespace clang