Warn on explicit copy constructors.
Summary:
The Google C++ Style Guide doesn't require copy constructors to be
declared explicit, but some people do this by mistake. Make this check detect
and fix such cases.
Reviewers: djasper
Reviewed By: djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D3541
llvm-svn: 207531
diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
index 944adf9..a150a8c 100644
--- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
@@ -15,6 +15,7 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/Support/raw_ostream.h"
@@ -28,16 +29,58 @@
Finder->addMatcher(constructorDecl().bind("ctor"), this);
}
+// Looks for the token matching the predicate and returns the range of the found
+// token including trailing whitespace.
+SourceRange FindToken(const SourceManager &Sources, LangOptions LangOpts,
+ SourceLocation StartLoc, SourceLocation EndLoc,
+ bool (*Pred)(const Token &)) {
+ FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc));
+ StringRef Buf = Sources.getBufferData(File);
+ const char *StartChar = Sources.getCharacterData(StartLoc);
+ Lexer Lex(StartLoc, LangOpts, StartChar, StartChar, Buf.end());
+ Lex.SetCommentRetentionState(true);
+ Token Tok;
+ do {
+ Lex.LexFromRawLexer(Tok);
+ if (Pred(Tok)) {
+ Token NextTok;
+ Lex.LexFromRawLexer(NextTok);
+ return SourceRange(Tok.getLocation(), NextTok.getLocation());
+ }
+ } while (Tok.isNot(tok::eof) && Tok.getLocation() < EndLoc);
+
+ return SourceRange();
+}
+
void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
const CXXConstructorDecl *Ctor =
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() ||
- Ctor->isDeleted() || Ctor->isCopyOrMoveConstructor())
+ if (Ctor->isOutOfLine() || Ctor->isImplicit() || Ctor->isDeleted())
return;
- if (Ctor->getNumParams() == 0 || Ctor->getMinRequiredArguments() > 1)
+
+ if (Ctor->isExplicit() && Ctor->isCopyOrMoveConstructor()) {
+ auto isKWExplicit = [](const Token &Tok) {
+ return Tok.is(tok::raw_identifier) &&
+ StringRef(Tok.getRawIdentifierData(), Tok.getLength()) ==
+ "explicit";
+ };
+ SourceRange ExplicitTokenRange =
+ FindToken(*Result.SourceManager, Result.Context->getLangOpts(),
+ Ctor->getOuterLocStart(), Ctor->getLocEnd(), isKWExplicit);
+ if (ExplicitTokenRange.isValid()) {
+ DiagnosticBuilder Diag = diag(ExplicitTokenRange.getBegin(),
+ "Copy constructor declared explicit.");
+ 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 ");
diff --git a/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
index b09afe2..3c024b1 100644
--- a/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
@@ -37,6 +37,16 @@
"class C { C(int i); }; C::C(int i) {}"));
}
+TEST(ExplicitConstructorCheckTest, RemoveExplicit) {
+ EXPECT_EQ("class A { A(const A&); };\n"
+ "class B { /*asdf*/ B(const B&); };\n"
+ "class C { /*asdf*/ C(const C&); };",
+ runCheckOnCode<ExplicitConstructorCheck>(
+ "class A { explicit A(const A&); };\n"
+ "class B { explicit /*asdf*/ B(const B&); };\n"
+ "class C { explicit/*asdf*/ C(const C&); };"));
+}
+
} // namespace test
} // namespace tidy
} // namespace clang