Fix assertion in google-explicit-constructor check when the constructor is

defined in a macro.

Summary:
We shouldn't suggest replacements in macros anyway, as we can't see all
usages of the macro and ensure the replacement is safe for all of them.

Reviewers: klimek

Reviewed By: klimek

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D3611

llvm-svn: 207987

2 files changed, 14 insertions, 3 deletions

diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
index a732242f778..7119ef0d3cb 100644
--- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
@@ -34,6 +34,8 @@ void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) {
 SourceRange FindToken(const SourceManager &Sources, LangOptions LangOpts,
                       SourceLocation StartLoc, SourceLocation EndLoc,
                       bool (*Pred)(const Token &)) {
+  if (StartLoc.isMacroID() || EndLoc.isMacroID())
+    return SourceRange();
   FileID File = Sources.getFileID(Sources.getSpellingLoc(StartLoc));
   StringRef Buf = Sources.getBufferData(File);
   const char *StartChar = Sources.getCharacterData(StartLoc);
@@ -69,10 +71,10 @@ void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
     SourceRange ExplicitTokenRange =
         FindToken(*Result.SourceManager, Result.Context->getLangOpts(),
                   Ctor->getOuterLocStart(), Ctor->getLocEnd(), isKWExplicit);
+    DiagnosticBuilder Diag =
+        diag(Ctor->getLocation(), "%0 constructor declared explicit.")
+        << (Ctor->isMoveConstructor() ? "Move" : "Copy");
     if (ExplicitTokenRange.isValid()) {
-      DiagnosticBuilder Diag = diag(ExplicitTokenRange.getBegin(),
-                                    "%0 constructor declared explicit.")
-                               << (Ctor->isMoveConstructor() ? "Move" : "Copy");
       Diag << FixItHint::CreateRemoval(
           CharSourceRange::getCharRange(ExplicitTokenRange));
     }
diff --git a/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
index d986d3bcb04..89e47407c98 100644
--- a/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/GoogleModuleTest.cpp
@@ -47,6 +47,15 @@ TEST(ExplicitConstructorCheckTest, RemoveExplicit) {
                 "class C { explicit/*asdf*/  C(const C&, int i = 0); };"));
 }
 
+TEST(ExplicitConstructorCheckTest, RemoveExplicitWithMacros) {
+  EXPECT_EQ(
+      "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
+      "A(Foo);",
+      runCheckOnCode<ExplicitConstructorCheck>(
+          "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
+          "A(Foo);"));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang