[libc++] Run clang-tidy in all configurations that are run in the Docker container

[philnik]

Fix diff

[Mordante]

libcxx/utils/ci/buildkite-pipeline.yml

326

The clang-tidy tests don't seem to be very cheap. I expect them to become more expensive in the future, since we probably want to add more tests. (I have some ideas on my own todo list.) So I wonder, does it have real benefit to enable them in all configurations? Especially this one. If it does then I think it would be good write the rationale in the commit message.

[philnik]

libcxx/utils/ci/buildkite-pipeline.yml

326

Most configurations have some code that's specific to it.

For me running clang-tidy takes ~40s. While that's not quick, it's also not exactly unbearable. We have other tests that take a lot longer. By far the slowest check is readability-identifier-naming, removing it brings the time down to ~8s. So I don't expect running clang-tidy to take that much longer in the future, since nasty_macros.compile.pass.cpp takes ~2s which is effectively just running the front-end. Since we run clang-tidy two times, all the checks except readability-identifier-naming take ~4s.

[ldionne]

libcxx/utils/ci/buildkite-pipeline.yml

326

At the risk of asking a dumb question, what's the benefit of running the checks in all configurations?

[philnik]

libcxx/utils/ci/buildkite-pipeline.yml

326

We cover code that is specific to the configuration. For example, the check that makes sure we use _Uglified attributes doesn't catch uses of __attribute__((trivial_abi)) currently. I only changed it because I ran clang-tidy with the unstable ABI locally. While I'm not aware of any TSan-specific code, there is code specific to ASan and UBSan.

[Mordante]

libcxx/utils/ci/buildkite-pipeline.yml

326

I see it takes about 80 s in the CI, not the cheapest, but not the most expensive either. I see the usefulness of different configurations, but I think some are not really useful.

AFAIK in GCC we disabled clang-tidy since there are conflicting compiler warnings, we also have very few work-arounds for older Clang versions, so there I too expect less warnings.

So I don't mind to enable it when we really expect different code paths to be validated, but we shouldn't blanket enable it.

[philnik]

libcxx/utils/ci/buildkite-pipeline.yml

326

I disagree. We wouldn't even have this discussion if we were able to just detect clang-tidy properly. We do this for no other slow test we have, so IMO we shouldn't treat it any different. This will just bite us in the future otherwise. I also don't think that 80s on a single thread makes much of a difference at large. This will increase the runtime by maybe two or three seconds per bot, which is a lot less than the variance we have between runs.

[ldionne]

libcxx/utils/ci/buildkite-pipeline.yml

326

Yeah, I agree, let's do it. Testing systematically is the simplest and the best approach, we should be addressing CI times as a general problem but not by disabling specific tests.

[ldionne]

Please make sure the CI is green though.

[philnik]

• Try to fix CI
• Rebased

[Mordante]

LGTM!

[philnik]

Try to fix CI

[philnik]

Try to fix CI

libcxx/include/__atomic/atomic_sync.h

 
 template <class _Tp>
 _LIBCPP_HIDE_FROM_ABI void __cxx_atomic_notify_all(__cxx_atomic_impl<_Tp> const volatile*) { }
 template <class _Tp>
 _LIBCPP_HIDE_FROM_ABI void __cxx_atomic_notify_one(__cxx_atomic_impl<_Tp> const volatile*) { }
 template <class _Atp, class _Fn>
 _LIBCPP_HIDE_FROM_ABI bool __cxx_atomic_wait(_Atp*, _Fn && __test_fn)
 {
-    return __libcpp_thread_poll_with_backoff(__test_fn, __spinning_backoff_policy());
+    return std::__libcpp_thread_poll_with_backoff(__test_fn, __spinning_backoff_policy());
 }
 
 #endif // _LIBCPP_HAS_NO_THREADS
 
 template <typename _Tp> _LIBCPP_HIDE_FROM_ABI
 bool __cxx_nonatomic_compare_equal(_Tp const& __lhs, _Tp const& __rhs) {
     return std::memcmp(std::addressof(__lhs), std::addressof(__rhs), sizeof(_Tp)) == 0;
 }

libcxx/include/__debug

 };
 
 template <class _Cont>
 inline bool
 _C_node<_Cont>::__dereferenceable(const void* __i) const
 {
     typedef typename _Cont::const_iterator iterator;
     const iterator* __j = static_cast<const iterator*>(__i);
-    _Cont* _Cp = static_cast<_Cont*>(__c_);
-    return _Cp->__dereferenceable(__j);
+    _Cont* __cp = static_cast<_Cont*>(__c_);
+    return __cp->__dereferenceable(__j);
 }
 
 template <class _Cont>
 inline bool
 _C_node<_Cont>::__decrementable(const void* __i) const
 {
     typedef typename _Cont::const_iterator iterator;
     const iterator* __j = static_cast<const iterator*>(__i);
-    _Cont* _Cp = static_cast<_Cont*>(__c_);
-    return _Cp->__decrementable(__j);
+    _Cont* __cp = static_cast<_Cont*>(__c_);
+    return __cp->__decrementable(__j);
 }
 
 template <class _Cont>
 inline bool
 _C_node<_Cont>::__addable(const void* __i, ptrdiff_t __n) const
 {
     typedef typename _Cont::const_iterator iterator;
     const iterator* __j = static_cast<const iterator*>(__i);
-    _Cont* _Cp = static_cast<_Cont*>(__c_);
-    return _Cp->__addable(__j, __n);
+    _Cont* __cp = static_cast<_Cont*>(__c_);
+    return __cp->__addable(__j, __n);
 }
 
 template <class _Cont>
 inline bool
 _C_node<_Cont>::__subscriptable(const void* __i, ptrdiff_t __n) const
 {
     typedef typename _Cont::const_iterator iterator;
     const iterator* __j = static_cast<const iterator*>(__i);
-    _Cont* _Cp = static_cast<_Cont*>(__c_);
-    return _Cp->__subscriptable(__j, __n);
+    _Cont* __cp = static_cast<_Cont*>(__c_);
+    return __cp->__subscriptable(__j, __n);
 }
 
 class _LIBCPP_TYPE_VIS __libcpp_db
 {
     __c_node** __cbeg_;
     __c_node** __cend_;
     size_t   __csz_;
     __i_node** __ibeg_;

libcxx/include/__tree

 //                nor any of its children refer to __z.  end_node->__left_
 //                may be different than the value passed in as __root.
 template <class _NodePtr>
 _LIBCPP_HIDE_FROM_ABI void
 __tree_remove(_NodePtr __root, _NodePtr __z) _NOEXCEPT
 {
     _LIBCPP_ASSERT(__root != nullptr, "Root node should not be null");
     _LIBCPP_ASSERT(__z != nullptr, "The node to remove should not be null");
-    _LIBCPP_DEBUG_ASSERT(__tree_invariant(__root), "The tree invariants should hold");
+    _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree invariants should hold");
     // __z will be removed from the tree.  Client still needs to destruct/deallocate it
     // __y is either __z, or if __z has two children, __tree_next(__z).
     // __y will have at most one child.
     // __y will be the initial hole in the tree (make the hole at a leaf)
     _NodePtr __y = (__z->__left_ == nullptr || __z->__right_ == nullptr) ?
                     __z : _VSTD::__tree_next(__z);
     // __x is __y's possibly null single child
     _NodePtr __x = __y->__left_ != nullptr ? __y->__left_ : __y->__right_;

libcxx/test/tools/clang_tidy_checks/CMakeLists.txt

 # Link only against clangTidy itself, not anything that clangTidy uses; otherwise we run setup code multiple times
 # which results in clang-tidy crashing
 set_target_properties(clangTidy PROPERTIES INTERFACE_LINK_LIBRARIES "")
 # ClangTargets.cmake doesn't set the include paths, so we have to do it
 target_include_directories(clangTidy INTERFACE
                            ${CLANG_INCLUDE_DIRS}
                            ${LLVM_INCLUDE_DIRS}
                           )
-target_compile_options(clangTidy INTERFACE -fno-rtti)
+target_compile_options(clangTidy INTERFACE
+                       -fno-rtti
+                       -fno-sanitize=address,hwaddress,undefined,thread,leak # ignore any sanitizers
+                      )
+
+if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+  target_compile_options(clangTidy INTERFACE
+                         -fno-sanitize=memory,dataflow
+                        )
+endif()
 
 add_library(cxx-tidy MODULE ${SOURCES})
 target_link_libraries(cxx-tidy clangTidy)
 
 set_target_properties(cxx-tidy PROPERTIES
                       CXX_STANDARD 20
                       CXX_STANDARD_REQUIRED YES
                       CXX_EXTENSIONS NO)
 
 set_target_properties(cxx-tidy PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 set(CMAKE_SHARED_MODULE_SUFFIX_CXX .plugin) # Use a portable suffix to simplify how we can find it from Lit

libcxx/utils/ci/buildkite-pipeline.yml

steps:
   - label: "GCC ${GCC_STABLE_VERSION} / C++latest"
     command: "libcxx/utils/ci/run-buildbot generic-gcc"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "gcc-${GCC_STABLE_VERSION}"
         CXX: "g++-${GCC_STABLE_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
steps:
   - label: "Modular build"
     command: "libcxx/utils/ci/run-buildbot generic-modules"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
steps:
   - label: "GCC ${GCC_STABLE_VERSION} / C++11"
     command: "libcxx/utils/ci/run-buildbot generic-gcc-cxx11"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "gcc-${GCC_STABLE_VERSION}"
         CXX: "g++-${GCC_STABLE_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Clang 15"
     command: "libcxx/utils/ci/run-buildbot generic-cxx2b"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-15"
         CXX: "clang++-15"
+        # TODO LLVM18: Enable clang-tidy
+        # ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Clang 16"
     command: "libcxx/utils/ci/run-buildbot generic-cxx2b"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-16"
         CXX: "clang++-16"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   # Tests with the sanitizers.
   - group: "Sanitizers"
     steps:
     - label: "ASAN"
       command: "libcxx/utils/ci/run-buildbot generic-asan"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "TSAN"
       command: "libcxx/utils/ci/run-buildbot generic-tsan"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "UBSAN"
       command: "libcxx/utils/ci/run-buildbot generic-ubsan"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "MSAN"
       command: "libcxx/utils/ci/run-buildbot generic-msan"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
steps:
   - label: "Static libraries"
     command: "libcxx/utils/ci/run-buildbot generic-static"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Shared library with merged ABI and unwinder libraries"
     command: "libcxx/utils/ci/run-buildbot generic-merged"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Assertions enabled"
     command: "libcxx/utils/ci/run-buildbot generic-assertions"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Debug mode"
     command: "libcxx/utils/ci/run-buildbot generic-debug-mode"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "With LLVM's libunwind"
     command: "libcxx/utils/ci/run-buildbot generic-with_llvm_unwinder"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - label: "Modular build with Local Submodule Visibility"
     command: "libcxx/utils/ci/run-buildbot generic-modules-lsv"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   - group: "Parts disabled"
     steps:
     - label: "No threads"
       command: "libcxx/utils/ci/run-buildbot generic-no-threads"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No filesystem"
       command: "libcxx/utils/ci/run-buildbot generic-no-filesystem"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No random device"
       command: "libcxx/utils/ci/run-buildbot generic-no-random_device"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No fstream"
       command: "libcxx/utils/ci/run-buildbot generic-no-fstream"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No locale"
       command: "libcxx/utils/ci/run-buildbot generic-no-localization"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No Unicode"
       command: "libcxx/utils/ci/run-buildbot generic-no-unicode"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No wide characters"
       command: "libcxx/utils/ci/run-buildbot generic-no-wide-characters"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No experimental features"
       command: "libcxx/utils/ci/run-buildbot generic-no-experimental"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
     - label: "No exceptions"
       command: "libcxx/utils/ci/run-buildbot generic-noexceptions"
       artifact_paths:
         - "**/test-results.xml"
         - "**/*.abilist"
       env:
           CC: "clang-${LLVM_HEAD_VERSION}"
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
         queue: "libcxx-builders"
         os: "linux"
       retry:
         automatic:
           - exit_status: -1  # Agent was lost
             limit: 2
       timeout_in_minutes: 120
 
   - label: "Unstable ABI"
     command: "libcxx/utils/ci/run-buildbot generic-abi-unstable"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120
 
   # Other non-testing CI jobs
   - label: "Benchmarks"
     command: "libcxx/utils/ci/run-buildbot benchmarks"
     artifact_paths:
       - "**/test-results.xml"
       - "**/*.abilist"
     env:
         CC: "clang-${LLVM_HEAD_VERSION}"
         CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
     agents:
       queue: "libcxx-builders"
       os: "linux"
     retry:
       automatic:
         - exit_status: -1  # Agent was lost
           limit: 2
     timeout_in_minutes: 120