[clang-tools-extra] 6a043ec - [clang-tidy] Fix ODR violation in unittests.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 30 08:53:30 PDT 2020


Author: Artem Dergachev
Date: 2020-07-30T08:52:47-07:00
New Revision: 6a043ecc0cf4d257d06c4fe0c3d5e1d9a8c7ea94

URL: https://github.com/llvm/llvm-project/commit/6a043ecc0cf4d257d06c4fe0c3d5e1d9a8c7ea94
DIFF: https://github.com/llvm/llvm-project/commit/6a043ecc0cf4d257d06c4fe0c3d5e1d9a8c7ea94.diff

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

Both tests define clang::tidy::test::TestCheck::registerMatchers().
This is UB and causes linker to sometimes choose the wrong overload.

Put classes into anonymous namespaces to avoid the problem.

Differential Revision: https://reviews.llvm.org/D84902

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
index a8729660bdce..0894e5fd5eb9 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -6,6 +6,7 @@ namespace clang {
 namespace tidy {
 namespace test {
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
@@ -20,17 +21,8 @@ class TestCheck : public ClangTidyCheck {
     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 @@ TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
-#endif
 
 } // namespace test
 } // namespace tidy

diff  --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
index c4239af0e767..bfa594098fb7 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -118,6 +118,7 @@ TEST(ParseConfiguration, MergeConfigurations) {
   EXPECT_TRUE(*Options.UseColor);
 }
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
@@ -140,6 +141,7 @@ class TestCheck : public ClangTidyCheck {
     return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...);
   }
 };
+} // namespace
 
 #define CHECK_VAL(Value, Expected)                                             \
   do {                                                                         \
@@ -222,9 +224,6 @@ TEST(CheckOptionsValidation, ValidIntOptions) {
 #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 @@ TEST(ValidConfiguration, ValidEnumOptions) {
 
 #undef CHECK_ERROR_ENUM
 }
-#endif
 
 #undef CHECK_VAL
 #undef CHECK_ERROR


        


More information about the cfe-commits mailing list