[PATCH] Fix assertion in google-explicit-constructor check when the constructor isdefined in a macro.

[Alexander Kornienko]

Hi klimek,

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.

http://reviews.llvm.org/D3611

Files:
  clang-tidy/google/GoogleTidyModule.cpp
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: clang-tidy/google/GoogleTidyModule.cpp
===================================================================
--- clang-tidy/google/GoogleTidyModule.cpp
+++ clang-tidy/google/GoogleTidyModule.cpp
@@ -34,6 +34,8 @@
 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 @@
     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));
     }
Index: unittests/clang-tidy/GoogleModuleTest.cpp
===================================================================
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -47,6 +47,15 @@
                 "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

Index: clang-tidy/google/GoogleTidyModule.cpp
===================================================================
--- clang-tidy/google/GoogleTidyModule.cpp
+++ clang-tidy/google/GoogleTidyModule.cpp
@@ -34,6 +34,8 @@
 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 @@
     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));
     }
Index: unittests/clang-tidy/GoogleModuleTest.cpp
===================================================================
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -47,6 +47,15 @@
                 "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

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits