[clang-tools-extra] r207987 - Fix assertion in google-explicit-constructor check when the constructor is

Alexander Kornienko alexfh at google.com
Mon May 5 11:49:32 PDT 2014


Author: alexfh
Date: Mon May  5 13:49:31 2014
New Revision: 207987

URL: http://llvm.org/viewvc/llvm-project?rev=207987&view=rev
Log:
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

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=207987&r1=207986&r2=207987&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Mon May  5 13:49:31 2014
@@ -34,6 +34,8 @@ void ExplicitConstructorCheck::registerM
 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(con
     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));
     }

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=207987&r1=207986&r2=207987&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp Mon May  5 13:49:31 2014
@@ -47,6 +47,15 @@ TEST(ExplicitConstructorCheckTest, Remov
                 "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





More information about the cfe-commits mailing list