[PATCH] D84902: [clang-tidy] Fix ODR violation in unittests.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr, dcoughlin, delcypher, yln, kubamracek, vsavchenko, njames93.
Herald added subscribers: martong, mgehre, Charusso, xazax.hun.
NoQ requested review of this revision.

I debugged the problem from D84453 <https://reviews.llvm.org/D84453> and D82188 <https://reviews.llvm.org/D82188>. The linker is not at fault - it's an ODR violation: there are two definitions for `clang::tidy::test::TestCheck::registerMatchers`. One test provides a non-trivial overload for `registerMatchers`, the other test doesn't overload this function (it only tests whether the check gets enabled when options are tweaked, so it doesn't register any matchers). When linker merges the two classes, the class without the overload may randomly win the race, and in this case the class that expects an overload appears to behave as if the function isn't called at all (because the definition in the superclass does nothing).

Put classes into anonymous namespaces to avoid the ODR violation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D84902

Files:
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -118,6 +118,7 @@
   EXPECT_TRUE(*Options.UseColor);
 }
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
@@ -140,6 +141,7 @@
     return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...);
   }
 };
+} // namespace
 
 #define CHECK_VAL(Value, Expected)                                             \
   do {                                                                         \
@@ -222,9 +224,6 @@
 #undef CHECK_ERROR_INT
 }
 
-// FIXME: Figure out why this test causes crashes on mac os.
-// See also comments around the ClangTidyDiagnosticConsumer.SortsErrors test.
-#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
   ClangTidyOptions Options;
@@ -276,7 +275,6 @@
 
 #undef CHECK_ERROR_ENUM
 }
-#endif
 
 #undef CHECK_VAL
 #undef CHECK_ERROR
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -6,6 +6,7 @@
 namespace tidy {
 namespace test {
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
@@ -20,17 +21,8 @@
     diag(Var->getTypeSpecStartLoc(), "type specifier");
   }
 };
+} // namespace
 
-// FIXME: This test seems to cause a strange linking interference
-// with the ValidConfiguration.ValidEnumOptions test on macOS.
-// If both tests are enabled, this test will fail as if
-// runCheckOnCode() is not invoked at all. Looks like a linker bug.
-// For now both tests are disabled on macOS. It is not sufficient
-// to only disable the other test because this test keeps failing
-// under Address Sanitizer, which may be an indication of more
-// such linking interference with other tests and this test
-// seems to be in the center of it.
-#ifndef __APPLE__
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector<ClangTidyError> Errors;
   runCheckOnCode<TestCheck>("int a;", &Errors);
@@ -38,7 +30,6 @@
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
-#endif
 
 } // namespace test
 } // namespace tidy


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84902.281775.patch
Type: text/x-patch
Size: 2634 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200730/c6b794a6/attachment.bin>