From 30f1ad15ca840fb76420b6ca393cf80c2553bf77 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Wed, 28 Oct 2026 17:50:51 -0400 Subject: [PATCH 01/12] clang-tidy as part of the actual build --- cuda_core/.clang-tidy | 70 +++++++++++++++++++ cuda_core/MANIFEST.in | 2 + cuda_core/build_hooks.py | 54 ++++++++++++++ cuda_core/cuda/core/_cpp/rt/context.cpp | 4 +- cuda_core/cuda/core/_cpp/rt/context_scope.hpp | 2 +- cuda_core/cuda/core/_cpp/rt/driver_api.cpp | 2 +- cuda_core/cuda/core/_cpp/rt/driver_api.hpp | 4 +- cuda_core/cuda/core/_cpp/rt/error.cpp | 10 +-- cuda_core/cuda/core/_cpp/rt/error.hpp | 8 +++ cuda_core/cuda/core/_cpp/rt/graph.cpp | 11 +-- cuda_core/cuda/core/_cpp/rt/graph_exec.cpp | 4 +- cuda_core/cuda/core/_cpp/rt/internal.hpp | 22 +++--- cuda_core/cuda/core/_cpp/rt/memory.cpp | 46 ++++++------ cuda_core/cuda/core/_cpp/rt/program.cpp | 3 +- cuda_core/cuda/core/_cpp/rt/py.hpp | 14 ++-- .../cuda/core/_cpp/rt/py_deferred_cleanup.cpp | 5 ++ cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp | 16 +++-- cuda_core/cuda/core/_cpp/rt/py_report.cpp | 2 +- cuda_core/cuda/core/_cpp/rt/stream.cpp | 8 +-- cuda_core/cuda/core/_cpp/rt/texture.cpp | 2 +- cuda_core/cuda/core/_cpp/rt/types.hpp | 24 +++++-- .../cuda/core/_cpp/rt/virtual_memory.cpp | 8 +-- cuda_core/pyproject.toml | 1 + cuda_core/setup.py | 27 ++++++- 24 files changed, 264 insertions(+), 85 deletions(-) create mode 100644 cuda_core/.clang-tidy diff --git a/cuda_core/.clang-tidy b/cuda_core/.clang-tidy new file mode 100644 index 00000000000..34146aa29db --- /dev/null +++ b/cuda_core/.clang-tidy @@ -0,0 +1,70 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +# Layout is clang-format's job (see .clang-format), so clang-tidy does no +# formatting of its own (FormatStyle: none below). Each enabled check is meant +# to be adoptable on its own, so findings can be fixed one check at a time. +# +# Checks are opt-in (-*) by group, minus the ones below. They cannot be +# commented inline because Checks is a single YAML string. +# +# Disabled because they conflict with the C-style CUDA driver API idioms: +# bugprone-easily-swappable-parameters: wrappers of driver calls take many +# adjacent parameters of the same type; the signatures mirror the API. +# bugprone-assignment-in-if-condition: `if (CUDA_SUCCESS != (err = CALL(...)))` +# is the standard error-handling pattern here. +# performance-no-int-to-ptr: opaque handles and file descriptors are +# converted to pointers with reinterpret_cast on purpose. +# modernize-avoid-c-arrays: arrays are passed to and filled by C APIs. +# modernize-return-braced-init-list: `return {};` vs. an explicit type is +# left to the author; it adds noise for the driver-API result structs. +# bugprone-throwing-static-initialization: the handle registries are +# process-lifetime globals whose constructors allocate on some standard +# libraries (MSVC); running out of memory while loading the module is +# fatal either way. +# +# Disabled because they do not match the codebase style: +# performance-enum-size: enum underlying types are left as they are. +# misc-const-correctness: const on every local is not the convention here. +# readability-function-cognitive-complexity: too disruptive. +# readability-identifier-length: short names such as the handle parameter `h` +# are used throughout. +# readability-implicit-bool-conversion: `if (ptr)` and `if (handle)` are the +# idiom for pointers and handles. +# misc-include-cleaner: too noisy with the CUDA and CPython umbrella headers. +# misc-no-recursion: recursion is used deliberately. +# misc-use-anonymous-namespace: file-local functions use `static`. +# modernize-use-trailing-return-type: the code uses `Type\nname()` layout. +Checks: > + -*, + bugprone-*, + -bugprone-easily-swappable-parameters, + -bugprone-assignment-in-if-condition, + -bugprone-throwing-static-initialization, + clang-analyzer-*, + misc-*, + -misc-include-cleaner, + -misc-const-correctness, + -misc-no-recursion, + -misc-use-anonymous-namespace, + modernize-*, + -modernize-use-trailing-return-type, + -modernize-avoid-c-arrays, + -modernize-return-braced-init-list, + performance-*, + -performance-enum-size, + -performance-no-int-to-ptr, + readability-*, + -readability-function-cognitive-complexity, + -readability-identifier-length, + -readability-implicit-bool-conversion +FormatStyle: none +CheckOptions: + # Records whose members are all public (the *Box and guard structs) are plain + # data holders; only a mix of public and private members is flagged. + misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: true +WarningsAsErrors: '*' +# Only report diagnostics for our own headers, not CUDA/CPython/libstdc++. +HeaderFilterRegex: 'cuda/core/_(cpp|include)/' +ExcludeHeaderFilterRegex: 'cuda/core/_include/(dlpack|aoti_shim)\.h' diff --git a/cuda_core/MANIFEST.in b/cuda_core/MANIFEST.in index f2f22eda4ff..91e3aa3bc4b 100644 --- a/cuda_core/MANIFEST.in +++ b/cuda_core/MANIFEST.in @@ -6,6 +6,8 @@ recursive-include cuda/core *.pyx *.pxd *.pxi *.pyi recursive-include cuda/core/_cpp *.cpp *.h *.hpp recursive-include cuda/core/_include *.h *.hpp *.def include build_hooks.py +# configuration of the clang-tidy run in the build +include .clang-tidy include cuda/core/py.typed include NOTICE # package-local shared PEP 517 helper diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index 46a5f70602c..6a7df8f999f 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -12,6 +12,7 @@ import importlib.util import os import re +import subprocess import sys import tempfile import zipfile @@ -704,6 +705,59 @@ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): return wheel_name +def _find_clang_tidy() -> str: + """The clang-tidy executable of the clang-tidy wheel in the build environment. + + The wheel is a build-system requirement in pyproject.toml. + + It is located through the wheel's ``clang_tidy`` package, never PATH, so a + system clang-tidy cannot be picked up. Raises RuntimeError when the wheel is + not installed. + """ + spec = importlib.util.find_spec("clang_tidy") + if spec is not None and spec.submodule_search_locations is not None: + exe = "clang-tidy" + if sys.platform == "win32": + exe = "clang-tidy.exe" + for base in spec.submodule_search_locations: + candidate = Path(base) / "data" / "bin" / exe + if candidate.is_file(): + return str(candidate) + raise RuntimeError( + "The clang-tidy build requirement of pyproject.toml is not installed. Isolated builds install it " + "automatically. For other builds, install the pinned version of it." + ) + + +def _is_handwritten_cpp(path) -> bool: + """Whether path is a handwritten C++ source, not a Cython-generated one.""" + path = Path(path) + return path.suffix == ".cpp" and path.resolve().is_relative_to(_PACKAGE_DIR / "_cpp") + + +def check_with_clang_tidy(compile_command) -> None: + """Run clang-tidy on the handwritten .cpp that a compiler command line compiles. + + ``compile_command`` is the exact command that setuptools is about to spawn, + so clang-tidy sees the same compiler, include directories, macros and + flags as the build. Commands that do not compile a handwritten source are + ignored. The configuration is cuda_core/.clang-tidy, which turns every + finding into an error. Raises RuntimeError with the findings. + """ + sources = [arg for arg in compile_command if _is_handwritten_cpp(arg)] + if not sources: + return + (source,) = sources + config = Path(__file__).resolve().parent / ".clang-tidy" + result = subprocess.run( # noqa: S603 + [_find_clang_tidy(), f"--config-file={config}", "--quiet", source, "--", *compile_command], + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError(f"clang-tidy reported errors in {source}:\n{result.stdout}{result.stderr}") + + def _get_cuda_bindings_require(): """The cuda-bindings build requirement for isolated builds. diff --git a/cuda_core/cuda/core/_cpp/rt/context.cpp b/cuda_core/cuda/core/_cpp/rt/context.cpp index 0e862e60864..fd632991be7 100644 --- a/cuda_core/cuda/core/_cpp/rt/context.cpp +++ b/cuda_core/cuda/core/_cpp/rt/context.cpp @@ -125,7 +125,7 @@ struct GreenCtxBox { CUgreenCtx resource; }; -static const ContextBox * +const ContextBox * get_box(const ContextHandle &h) noexcept { const CUcontext *p = h.get(); @@ -133,7 +133,7 @@ get_box(const ContextHandle &h) noexcept } // See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) -static HandleRegistry context_registry; +HandleRegistry context_registry; // Create a context handle reference, with optional green context as source. ContextHandle diff --git a/cuda_core/cuda/core/_cpp/rt/context_scope.hpp b/cuda_core/cuda/core/_cpp/rt/context_scope.hpp index cf0feb8db04..c0ce6c7542a 100644 --- a/cuda_core/cuda/core/_cpp/rt/context_scope.hpp +++ b/cuda_core/cuda/core/_cpp/rt/context_scope.hpp @@ -126,7 +126,7 @@ cleanup_in_context( } std::invoke(std::forward(after_cleanup)); if (status != CUDA_SUCCESS || restore != CUDA_SUCCESS) { - char operation_name[160]; + char operation_name[kOperationNameSize]; format_operation(operation_name, sizeof(operation_name), name, handle); if (status != CUDA_SUCCESS) { report_cuda_error(operation_name, status, detail); diff --git a/cuda_core/cuda/core/_cpp/rt/driver_api.cpp b/cuda_core/cuda/core/_cpp/rt/driver_api.cpp index 752076af6d8..a489bb5231b 100644 --- a/cuda_core/cuda/core/_cpp/rt/driver_api.cpp +++ b/cuda_core/cuda/core/_cpp/rt/driver_api.cpp @@ -9,7 +9,7 @@ namespace cuda_core::rt { // The pointers. Null until ensure_fn_table() fills the table. See driver_api.hpp. -#define CUDA_CORE_DEFINE_DRIVER_FN(name, introduced) decltype(&name) p_##name = nullptr; +#define CUDA_CORE_DEFINE_DRIVER_FN(name, introduced) decltype(&(name)) p_##name = nullptr; CUDA_CORE_DRIVER_FUNCTIONS(CUDA_CORE_DEFINE_DRIVER_FN) #undef CUDA_CORE_DEFINE_DRIVER_FN diff --git a/cuda_core/cuda/core/_cpp/rt/driver_api.hpp b/cuda_core/cuda/core/_cpp/rt/driver_api.hpp index d759112385e..1f0ec0721f6 100644 --- a/cuda_core/cuda/core/_cpp/rt/driver_api.hpp +++ b/cuda_core/cuda/core/_cpp/rt/driver_api.hpp @@ -141,7 +141,7 @@ namespace cuda_core::rt { X(cuStreamIsCapturing, 10000) \ X(cuThreadExchangeStreamCaptureMode, 10010) -#define CUDA_CORE_DECLARE_DRIVER_FN(name, introduced) extern decltype(&name) p_##name; +#define CUDA_CORE_DECLARE_DRIVER_FN(name, introduced) extern decltype(&(name)) p_##name; CUDA_CORE_DRIVER_FUNCTIONS(CUDA_CORE_DECLARE_DRIVER_FN) #undef CUDA_CORE_DECLARE_DRIVER_FN @@ -229,7 +229,7 @@ struct Unavailable; template struct Unavailable { static R - call(A...) noexcept + call(A... /*unused*/) noexcept { return UnavailableStatus::value; } diff --git a/cuda_core/cuda/core/_cpp/rt/error.cpp b/cuda_core/cuda/core/_cpp/rt/error.cpp index 72402122a09..97f2877bb69 100644 --- a/cuda_core/cuda/core/_cpp/rt/error.cpp +++ b/cuda_core/cuda/core/_cpp/rt/error.cpp @@ -27,8 +27,8 @@ namespace { // Thread-local detail attached to the next raised CUDAError with a matching // status (see take_last_error_detail()). Written only by propagating helpers. // The taken copy stays valid until the next take on the same thread. -thread_local char last_error_detail[512] = {0}; -thread_local char taken_error_detail[512] = {0}; +thread_local char last_error_detail[kErrorMessageSize] = {0}; +thread_local char taken_error_detail[kErrorMessageSize] = {0}; thread_local CUresult last_error_detail_status = CUDA_SUCCESS; } // namespace @@ -66,7 +66,7 @@ format_operation(char *buffer, size_t size, const char *operation, unsigned long void report_status_code(const char *operation, long code) noexcept { - char message[256]; + char message[kErrorMessageSize]; std::snprintf(message, sizeof(message), "%s failed (status %ld)", operation, code); report_message(message); } @@ -80,7 +80,7 @@ report_cuda_error(const char *operation, CUresult status, const char *detail) no if (status == CUDA_SUCCESS || status == CUDA_ERROR_DEINITIALIZED) { return; } - char message[512]; + char message[kErrorMessageSize]; format_cuda_error(message, sizeof(message), operation, status, detail); report_message(message); } @@ -122,7 +122,7 @@ note_context_not_restored(CUcontext previous, CUresult operation_status, CUresul if (DRIVER_CALL(cuCtxGetCurrent, ¤t) != CUDA_SUCCESS) { current = nullptr; } - char cause[128] = {0}; + char cause[kErrorMessageSize] = {0}; if (operation_status != CUDA_SUCCESS) { const char *error_name = nullptr; if (DRIVER_CALL(cuGetErrorName, restore_status, &error_name) == CUDA_SUCCESS) { diff --git a/cuda_core/cuda/core/_cpp/rt/error.hpp b/cuda_core/cuda/core/_cpp/rt/error.hpp index 9d975f4aa41..251e7388f0c 100644 --- a/cuda_core/cuda/core/_cpp/rt/error.hpp +++ b/cuda_core/cuda/core/_cpp/rt/error.hpp @@ -5,9 +5,17 @@ #pragma once #include +#include namespace cuda_core::rt { +// Sizes of the stack buffers that format error text. Messages are truncated by +// snprintf, never overrun. +constexpr std::size_t kOperationNameSize = 160; // " (handle 0x...)" from format_operation() +constexpr std::size_t kErrorMessageSize = 512; // a full error message or error detail +// A message that embeds one error message after some fixed text. +constexpr std::size_t kNestedMessageSize = kErrorMessageSize + (kErrorMessageSize / 2); + // ============================================================================ // Thread-local error handling // ============================================================================ diff --git a/cuda_core/cuda/core/_cpp/rt/graph.cpp b/cuda_core/cuda/core/_cpp/rt/graph.cpp index 13720e6a9b1..4d8905c84e4 100644 --- a/cuda_core/cuda/core/_cpp/rt/graph.cpp +++ b/cuda_core/cuda/core/_cpp/rt/graph.cpp @@ -83,9 +83,10 @@ struct GraphBox : GraphBoxBase { GraphAttachmentMap attachments; // Non-owning attachment index. HandleRegistry node_handles; + // Not noexcept: the std::map and HandleRegistry members allocate. GraphBox( CUgraph resource_, GraphHierarchy *hierarchy_, GraphBox *parent_ = nullptr, CUgraphNode owner_node_ = nullptr - ) noexcept + ) : GraphBoxBase{resource_}, hierarchy(hierarchy_), parent(parent_), owner_node(owner_node_) { } @@ -106,7 +107,7 @@ struct GraphHierarchy { // See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) using GraphRegistry = HandleRegistry; -static GraphRegistry graph_registry; +GraphRegistry graph_registry; // Immutable resource owners for one version of a graph node's parameters. // Inheriting DeferredCleanupItem lets CUDA's user-object destructor enqueue @@ -140,7 +141,7 @@ free_deleter(const void *p) noexcept GraphBox * get_box(const GraphHandle &h) noexcept { - auto *value = reinterpret_cast(h.get()); + const auto *value = reinterpret_cast(h.get()); return const_cast(static_cast(value)); } @@ -254,7 +255,7 @@ struct PreparedAttachmentState { explicit PreparedAttachmentState(GraphHandle h_graph_) : h_graph(std::move(h_graph_)) {} }; -void +static void rollback_prepared_attachment(PreparedAttachmentState *state) noexcept { if (!state) { @@ -389,7 +390,7 @@ graph_prepare_child_graph_update( return CUDA_SUCCESS; } -void +static void publish_child_graph_update(PreparedChildGraphUpdateState &state, GraphHandle *out_child) { GraphBox *parent = get_box(state.h_parent); diff --git a/cuda_core/cuda/core/_cpp/rt/graph_exec.cpp b/cuda_core/cuda/core/_cpp/rt/graph_exec.cpp index b78730e7e4a..9b54a6cec22 100644 --- a/cuda_core/cuda/core/_cpp/rt/graph_exec.cpp +++ b/cuda_core/cuda/core/_cpp/rt/graph_exec.cpp @@ -92,7 +92,7 @@ struct ExecAttachmentStaging { if (!h_source || !accumulator) { return CUDA_SUCCESS; } - const CUuserObject object = accumulator->object; + CUuserObject object = accumulator->object; const GraphHandle source = std::move(h_source); accumulator = nullptr; GILReleaseGuard gil; @@ -154,7 +154,7 @@ struct PreparedExecAttachmentState { } }; -void +static void rollback_prepared_exec_attachment(PreparedExecAttachmentState *state) noexcept { if (!state) { diff --git a/cuda_core/cuda/core/_cpp/rt/internal.hpp b/cuda_core/cuda/core/_cpp/rt/internal.hpp index 270ee486aa2..56c935b0103 100644 --- a/cuda_core/cuda/core/_cpp/rt/internal.hpp +++ b/cuda_core/cuda/core/_cpp/rt/internal.hpp @@ -39,7 +39,7 @@ handle_bits(const T &value) noexcept if constexpr (std::is_pointer_v) { using P = std::remove_cv_t>; if constexpr (std::is_pointer_v

) { - return value ? handle_bits(*value) : 0ull; + return value ? handle_bits(*value) : 0ULL; } else { return static_cast(reinterpret_cast(value)); @@ -49,14 +49,14 @@ handle_bits(const T &value) noexcept return static_cast(value); } else { - return 0ull; + return 0ULL; } } // Store a stream and any state needed to preserve deallocation ordering. struct DeallocationStream { StreamHandle h_stream; - std::thread::id ptds_tid{}; + std::thread::id ptds_tid; }; // Implemented in stream.cpp @@ -110,7 +110,7 @@ class WarnOnFailure { if (status == CUDA_SUCCESS || status == CUDA_ERROR_DEINITIALIZED) { return; } - char operation[160]; + char operation[kOperationNameSize]; format_operation(operation, sizeof(operation), operation_, handle); report_cuda_error(operation, status); } @@ -120,7 +120,7 @@ class WarnOnFailure { report(Status status, unsigned long long handle) const noexcept { if (static_cast(status) != 0) { - char operation[160]; + char operation[kOperationNameSize]; format_operation(operation, sizeof(operation), operation_, handle); report_status_code(operation, static_cast(status)); } @@ -182,21 +182,21 @@ class HandleRegistry { void register_handle(const Key &key, const Handle &h) { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); map_[key] = h; } void unregister_handle(const Key &key) noexcept { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); map_.erase(key); } void register_handles(const std::vector &handles) { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); for (const Handle &h : handles) { if (h) { map_[*h] = h; @@ -207,7 +207,7 @@ class HandleRegistry { Handle lookup(const Key &key) { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); auto it = map_.find(key); if (it != map_.end()) { if (auto h = it->second.lock()) { @@ -222,7 +222,7 @@ class HandleRegistry { Handle get_or_create(const Key &key, Factory &&create) { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); auto it = map_.find(key); if (it != map_.end()) { if (Handle h = it->second.lock()) { @@ -241,7 +241,7 @@ class HandleRegistry { MapType drain() noexcept { - std::lock_guard lock(mutex_); + std::scoped_lock lock(mutex_); MapType extracted; extracted.swap(map_); return extracted; diff --git a/cuda_core/cuda/core/_cpp/rt/memory.cpp b/cuda_core/cuda/core/_cpp/rt/memory.cpp index f280aeedc5b..d6b4dd5754f 100644 --- a/cuda_core/cuda/core/_cpp/rt/memory.cpp +++ b/cuda_core/cuda/core/_cpp/rt/memory.cpp @@ -68,7 +68,7 @@ clear_mempool_peer_access(CUmemoryPool pool, int owner_device) noexcept DRIVER_CALL(cuMemPoolSetAccess, pool, &revoke, 1); // Best effort } } - catch (...) { + catch (...) { // NOLINT(bugprone-empty-catch) // Swallow exceptions - this is best-effort cleanup in destructor context } } @@ -139,7 +139,7 @@ create_mempool_handle_ipc(int fd, CUmemAllocationHandleType handle_type) { GILReleaseGuard gil; CUmemoryPool pool; - auto handle_ptr = reinterpret_cast(static_cast(fd)); + auto *handle_ptr = reinterpret_cast(static_cast(fd)); if (CUDA_SUCCESS != (err = DRIVER_CALL(cuMemPoolImportFromShareableHandle, &pool, handle_ptr, handle_type, 0))) { return {}; } @@ -513,7 +513,7 @@ struct ExportDataKeyHash { std::size_t h = 0; const auto *bytes = reinterpret_cast(&key.data); for (std::size_t i = 0; i < sizeof(key.data); ++i) { - h = h * 31 + bytes[i]; + h = (h * 31) + bytes[i]; // NOLINT(readability-magic-numbers) } return h; } @@ -527,7 +527,7 @@ static std::mutex ipc_import_mutex; DevicePtrHandle deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, const StreamHandle &h_stream) { - auto data = const_cast(reinterpret_cast(export_data)); + auto *data = const_cast(reinterpret_cast(export_data)); // Resolve the table before you take any lock: a fill acquires the GIL, and // nothing under ipc_import_mutex may acquire it (#2840). The raw p_ calls @@ -552,7 +552,7 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co CUresult discard_status = CUDA_SUCCESS; CUdeviceptr discarded = 0; { - std::lock_guard lock(ipc_import_mutex); + std::scoped_lock lock(ipc_import_mutex); if (auto h = ipc_ptr_cache.lookup(key)) { return h; @@ -586,7 +586,7 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co [&]() noexcept { return p_cuMemFreeAsync(b->resource, as_cu(stream.h_stream)); // raw: under ipc_import_mutex }, - [&]() noexcept { lock.unlock(); }); + [&]() noexcept { lock.release()->unlock(); }); delete b; } ); @@ -602,7 +602,7 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co discarded = ptr; } if (discard_status != CUDA_SUCCESS) { - char operation[160]; + char operation[kOperationNameSize]; format_operation(operation, sizeof(operation), "cuMemFreeAsync", discarded); report_cuda_error( operation, @@ -613,20 +613,19 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co } return {}; } - else { - GILReleaseGuard gil; - CUdeviceptr ptr; - if (CUDA_SUCCESS != (err = DRIVER_CALL(cuMemPoolImportPointer, &ptr, *h_pool, data))) { - return {}; - } + GILReleaseGuard gil; + CUdeviceptr ptr; + if (CUDA_SUCCESS != (err = DRIVER_CALL(cuMemPoolImportPointer, &ptr, *h_pool, data))) { + return {}; + } - DeallocationStream ds; - if (!make_deallocation_stream(h_stream, ds)) { - pw_cuMemFreeAsync(ptr, as_cu(h_stream)); - return {}; - } + DeallocationStream ds; + if (!make_deallocation_stream(h_stream, ds)) { + pw_cuMemFreeAsync(ptr, as_cu(h_stream)); + return {}; + } - // clang-format off + // clang-format off auto box = std::shared_ptr( new DevicePtrBox{ptr, std::move(ds)}, [h_pool](DevicePtrBox *b) { @@ -641,9 +640,8 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co delete b; } ); - // clang-format on - return DevicePtrHandle(box, &box->resource); - } + // clang-format on + return DevicePtrHandle(box, &box->resource); } // ============================================================================ @@ -651,7 +649,7 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co // ============================================================================ FileDescriptorHandle -create_fd_handle(int fd) +create_fd_handle([[maybe_unused]] int fd) { #ifdef _WIN32 throw std::runtime_error("create_fd_handle is not supported on Windows"); @@ -671,7 +669,7 @@ create_fd_handle(int fd) } FileDescriptorHandle -create_fd_handle_ref(int fd) +create_fd_handle_ref([[maybe_unused]] int fd) { #ifdef _WIN32 throw std::runtime_error("create_fd_handle_ref is not supported on Windows"); diff --git a/cuda_core/cuda/core/_cpp/rt/program.cpp b/cuda_core/cuda/core/_cpp/rt/program.cpp index 4186b73df76..465a94b0df9 100644 --- a/cuda_core/cuda/core/_cpp/rt/program.cpp +++ b/cuda_core/cuda/core/_cpp/rt/program.cpp @@ -129,8 +129,9 @@ create_kernel_handle_ref(CUkernel kernel) LibraryHandle get_kernel_library(const KernelHandle &h) noexcept { - if (!h) + if (!h) { return {}; + } return get_box(h)->h_library; } diff --git a/cuda_core/cuda/core/_cpp/rt/py.hpp b/cuda_core/cuda/core/_cpp/rt/py.hpp index b3e599189fa..cc8c2f61226 100644 --- a/cuda_core/cuda/core/_cpp/rt/py.hpp +++ b/cuda_core/cuda/core/_cpp/rt/py.hpp @@ -72,7 +72,7 @@ class GILReleaseGuard { // Use in C++ destructors that need to manipulate Python objects. class GILAcquireGuard { public: - GILAcquireGuard() : acquired_(false) + GILAcquireGuard() { // Don't try to acquire GIL if Python is finalizing if (!Py_IsInitialized() || py_is_finalizing()) { @@ -89,7 +89,7 @@ class GILAcquireGuard { } } - bool + [[nodiscard]] bool acquired() const { return acquired_; @@ -100,8 +100,8 @@ class GILAcquireGuard { GILAcquireGuard &operator=(const GILAcquireGuard &) = delete; private: - PyGILState_STATE gstate_; - bool acquired_; + PyGILState_STATE gstate_{}; + bool acquired_{false}; }; // Save the Python exception in flight and restore it on scope exit, dropping @@ -155,12 +155,14 @@ make_py(const char *module_name, const char *class_name, std::intptr_t value) no Py_RETURN_NONE; } PyObject *mod = PyImport_ImportModule(module_name); - if (!mod) + if (!mod) { return nullptr; + } PyObject *cls = PyObject_GetAttrString(mod, class_name); Py_DECREF(mod); - if (!cls) + if (!cls) { return nullptr; + } PyObject *result = PyObject_CallFunction(cls, "L", value); Py_DECREF(cls); return result; diff --git a/cuda_core/cuda/core/_cpp/rt/py_deferred_cleanup.cpp b/cuda_core/cuda/core/_cpp/rt/py_deferred_cleanup.cpp index 4a3c75b995e..374c9390039 100644 --- a/cuda_core/cuda/core/_cpp/rt/py_deferred_cleanup.cpp +++ b/cuda_core/cuda/core/_cpp/rt/py_deferred_cleanup.cpp @@ -146,6 +146,10 @@ enqueue_cleanup(void *item) noexcept // Module initialization calls this once with the GIL held, which serializes // the check, allocation, and publication below. +// +// The queue lives for the rest of the process. MSVC's std::atomic hides the +// publication from the analyzer, which then reports a leak at the closing brace. +// NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks) void initialize_deferred_cleanup() { @@ -155,6 +159,7 @@ initialize_deferred_cleanup() auto *queue = new DeferredCleanupQueue(); deferred_cleanup_queue.store(queue, std::memory_order_release); } +// NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks) void retry_deferred_cleanup() noexcept diff --git a/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp b/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp index fd36385def8..c4ca537373a 100644 --- a/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp +++ b/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp @@ -46,7 +46,7 @@ std::atomic table_ready[kTables]; std::atomic table_failed[kTables]; std::atomic unavailable_reported[kTables]; std::mutex fill_mutex; -char fill_error[kTables][512] = {}; // guarded by fill_mutex +char fill_error[kTables][kErrorMessageSize] = {}; // guarded by fill_mutex std::size_t index_of(FnTable table) noexcept @@ -131,7 +131,7 @@ void record_failure(FnTable table, const char *message, bool latch = true) noexcept { { - std::lock_guard lock(fill_mutex); + std::scoped_lock lock(fill_mutex); std::snprintf(fill_error[index_of(table)], sizeof(fill_error[0]), "%s", message); } if (latch) { @@ -151,7 +151,7 @@ fn_table_ready(FnTable table) noexcept bool fn_table_error(FnTable table, char *buffer, std::size_t size) noexcept { - std::lock_guard lock(fill_mutex); + std::scoped_lock lock(fill_mutex); const char *text = fill_error[index_of(table)]; if (!text[0]) { return false; @@ -181,8 +181,8 @@ ensure_fn_table(FnTable table) noexcept return false; } - char message[640]; - char cause[384]; + char message[kNestedMessageSize]; + char cause[kErrorMessageSize]; void *values[kMaxEntries] = {}; GILAcquireGuard gil; @@ -270,6 +270,7 @@ ensure_fn_table(FnTable table) noexcept // major series is present in every driver cuda.core supports. A null one // means the driver is older than that. A newer function may be null, and // the Cython layer gates its use on the driver version. + // NOLINTBEGIN(readability-magic-numbers): version-number decoding, e.g. 13040 -> 13.4 if (table == FnTable::driver) { for (std::size_t i = 0; i < count; ++i) { if (values[i] == nullptr && entries[i].introduced <= CUDA_CORE_BUILD_MAJOR * 1000) { @@ -288,9 +289,10 @@ ensure_fn_table(FnTable table) noexcept } } } + // NOLINTEND(readability-magic-numbers) { - std::lock_guard lock(fill_mutex); + std::scoped_lock lock(fill_mutex); if (!table_ready[idx].load(std::memory_order_relaxed)) { for (std::size_t i = 0; i < count; ++i) { *entries[i].slot = values[i]; @@ -307,7 +309,7 @@ report_unavailable_fn(FnTable table, const char *name) noexcept { char reason[sizeof(fill_error[0])]; const bool failed_fill = fn_table_error(table, reason, sizeof(reason)); - char message[768]; + char message[kNestedMessageSize]; if (failed_fill) { std::snprintf(message, sizeof(message), "cuda.core could not call %s: %s", name, reason); } diff --git a/cuda_core/cuda/core/_cpp/rt/py_report.cpp b/cuda_core/cuda/core/_cpp/rt/py_report.cpp index 6bc24bf3a5c..3c0f3abf49c 100644 --- a/cuda_core/cuda/core/_cpp/rt/py_report.cpp +++ b/cuda_core/cuda/core/_cpp/rt/py_report.cpp @@ -104,7 +104,7 @@ attach_rollback_failure(const char *operation, CUresult status, const char *deta if (status == CUDA_SUCCESS || status == CUDA_ERROR_DEINITIALIZED) { return; } - char message[512]; + char message[kErrorMessageSize]; format_cuda_error(message, sizeof(message), operation, status, detail); if (!add_note_to_handled_exception(message)) { report_message(message); diff --git a/cuda_core/cuda/core/_cpp/rt/stream.cpp b/cuda_core/cuda/core/_cpp/rt/stream.cpp index eba71ca5be8..a0a037727b9 100644 --- a/cuda_core/cuda/core/_cpp/rt/stream.cpp +++ b/cuda_core/cuda/core/_cpp/rt/stream.cpp @@ -58,7 +58,7 @@ struct StreamBox { ContextHandle h_context; }; -static const StreamBox * +const StreamBox * get_box(const StreamHandle &h) noexcept { const CUstream *p = h.get(); @@ -66,7 +66,7 @@ get_box(const StreamHandle &h) noexcept } // See REGISTRY_DESIGN.md (Level 1: Driver Handle -> Resource Handle) -static HandleRegistry stream_registry; +HandleRegistry stream_registry; } // namespace StreamHandle @@ -224,7 +224,7 @@ make_deallocation_stream(const StreamHandle &h, DeallocationStream &out) noexcep return true; } - const CUstream stream = as_cu(h); + CUstream stream = as_cu(h); if (!is_default_stream(stream)) { out = DeallocationStream{h, {}}; return true; @@ -241,7 +241,7 @@ make_deallocation_stream(const StreamHandle &h, DeallocationStream &out) noexcep } // Do not register in stream_registry: the token value alone is not // a unique stream identity (context is part of the meaning). - auto box = std::shared_ptr(new StreamBox{stream, h_ctx}); + auto box = std::make_shared(StreamBox{stream, h_ctx}); h_bound = StreamHandle(box, &box->resource); } diff --git a/cuda_core/cuda/core/_cpp/rt/texture.cpp b/cuda_core/cuda/core/_cpp/rt/texture.cpp index 1e2de1e2db8..cbbe7e0eff8 100644 --- a/cuda_core/cuda/core/_cpp/rt/texture.cpp +++ b/cuda_core/cuda/core/_cpp/rt/texture.cpp @@ -101,7 +101,7 @@ get_box(const MipmappedArrayHandle &h) noexcept } // Wrap an array with shared owning-destruction behavior. -static OpaqueArrayHandle +OpaqueArrayHandle wrap_array_owned(CUarray arr, ContextHandle h_context) { // clang-format off diff --git a/cuda_core/cuda/core/_cpp/rt/types.hpp b/cuda_core/cuda/core/_cpp/rt/types.hpp index 08e5bb65e49..487aeaf3f9f 100644 --- a/cuda_core/cuda/core/_cpp/rt/types.hpp +++ b/cuda_core/cuda/core/_cpp/rt/types.hpp @@ -34,23 +34,33 @@ struct TaggedHandle { T raw; }; -using NvvmProgramValue = TaggedHandle; -using NvJitLinkValue = TaggedHandle; +enum HandleTag : int { + kTagNvvmProgram, + kTagNvJitLink, + kTagTexObject, + kTagSurfObject, + kTagMemAllocation, + kTagVaReservation, + kTagVaMapping, +}; + +using NvvmProgramValue = TaggedHandle; +using NvJitLinkValue = TaggedHandle; // CUtexObject, CUsurfObject and CUdeviceptr are all `unsigned long long`, so // shared_ptr et al. would be the *same* C++ type as // DevicePtrHandle (and each other), collapsing the as_cu/as_intptr/as_py // overload sets. Tag them to keep each handle type distinct, exactly as the // NVVM / nvJitLink handles above do. -using TexObjectValue = TaggedHandle; -using SurfObjectValue = TaggedHandle; +using TexObjectValue = TaggedHandle; +using SurfObjectValue = TaggedHandle; // Virtual memory management (VMM_DESIGN.md). CUmemGenericAllocationHandle is // also `unsigned long long`, and the reservation and mapping handles carry a // CUdeviceptr each, so all three are tagged for the same reason. -using MemAllocationValue = TaggedHandle; -using VaReservationValue = TaggedHandle; -using VaMappingValue = TaggedHandle; +using MemAllocationValue = TaggedHandle; +using VaReservationValue = TaggedHandle; +using VaMappingValue = TaggedHandle; // ============================================================================ // Handle type aliases - expose only the raw CUDA resource diff --git a/cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp b/cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp index ea8c1490a4f..ed54a596139 100644 --- a/cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp +++ b/cuda_core/cuda/core/_cpp/rt/virtual_memory.cpp @@ -186,8 +186,8 @@ create_va_mapping_handle(CUdeviceptr ptr, const MemAllocationHandle &h_alloc, co err = CUDA_ERROR_INVALID_VALUE; return {}; } - const MemAllocationBox *alloc = box_of(h_alloc); - const VaReservationBox *res = box_of(h_res); + const auto *alloc = box_of(h_alloc); + const auto *res = box_of(h_res); const CUdeviceptr base = res->resource.raw; if (ptr < base || ptr - base > res->size || alloc->size > res->size - (ptr - base)) { err = CUDA_ERROR_INVALID_VALUE; @@ -270,7 +270,7 @@ sync_would_disturb_capture(CUstream stream) noexcept static void sync_recorded_stream(const DeallocationStream &ds, bool &capture_skipped) noexcept { - const CUstream s = as_cu(ds.h_stream); + CUstream s = as_cu(ds.h_stream); CUcontext previous = nullptr; int changed = 0; const char *detail = nullptr; @@ -297,7 +297,7 @@ sync_recorded_stream(const DeallocationStream &ds, bool &capture_skipped) noexce clear_last_error_detail(); } if (status != CUDA_SUCCESS || restore != CUDA_SUCCESS) { - char operation_name[160]; + char operation_name[kOperationNameSize]; format_operation(operation_name, sizeof(operation_name), "cuStreamSynchronize", handle_bits(s)); if (status != CUDA_SUCCESS) { report_cuda_error(operation_name, status, detail); diff --git a/cuda_core/pyproject.toml b/cuda_core/pyproject.toml index e850dadacfe..705da894c5c 100644 --- a/cuda_core/pyproject.toml +++ b/cuda_core/pyproject.toml @@ -8,6 +8,7 @@ requires = [ "setuptools-scm[simple]>=8,!=10.1", "Cython>=3.2.5,<3.3", "cuda-pathfinder>=1.5", + "clang-tidy==22.1.8", ] build-backend = "build_hooks" backend-path = ["."] diff --git a/cuda_core/setup.py b/cuda_core/setup.py index 795c80546aa..af8b5b193cf 100644 --- a/cuda_core/setup.py +++ b/cuda_core/setup.py @@ -86,6 +86,31 @@ def _configure_windows_tensor_bridge(self): raise RuntimeError(f"Failed to find extension {_TENSOR_BRIDGE_EXT_NAME!r} for Windows build.") + @contextlib.contextmanager + def _clang_tidy_before_compile(self): + """Run clang-tidy on each handwritten .cpp just before its compiler command runs. + + Every compiler, MSVC included, runs its commands through one method, so + clang-tidy gets the exact command line of the build. That method is + call() in current setuptools, where spawn() is deprecated and delegates + to it, and spawn() before. Only sources that are rebuilt are checked. + """ + compiler = self.compiler + method = "spawn" + if hasattr(compiler, "call"): + method = "call" + stock = getattr(compiler, method) + + def run(cmd, *args, **kwargs): + build_hooks.check_with_clang_tidy(cmd) + return stock(cmd, *args, **kwargs) + + setattr(compiler, method, run) + try: + yield + finally: + setattr(compiler, method, stock) + @contextlib.contextmanager def _parallel_source_compilation(self): """Compile the sources of every extension through one shared thread pool. @@ -140,7 +165,7 @@ def compile_one(obj): def build_extensions(self): self.parallel = nthreads self._configure_windows_tensor_bridge() - with self._parallel_source_compilation(): + with self._clang_tidy_before_compile(), self._parallel_source_compilation(): super().build_extensions() From 8ad40301f85e52e155e9afc8083623d9f096b222 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 10:30:50 -0400 Subject: [PATCH 02/12] Address comments in the review, and make clang-tidy work in the pixi build --- cuda_core/.clang-tidy | 4 +- cuda_core/build_hooks.py | 120 +++++++++++++++++++++++------ cuda_core/cuda/core/_cpp/rt/py.hpp | 2 +- cuda_core/pixi.toml | 1 + cuda_core/pyproject.toml | 2 +- 5 files changed, 102 insertions(+), 27 deletions(-) diff --git a/cuda_core/.clang-tidy b/cuda_core/.clang-tidy index 34146aa29db..b4f428d2407 100644 --- a/cuda_core/.clang-tidy +++ b/cuda_core/.clang-tidy @@ -66,5 +66,5 @@ CheckOptions: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: true WarningsAsErrors: '*' # Only report diagnostics for our own headers, not CUDA/CPython/libstdc++. -HeaderFilterRegex: 'cuda/core/_(cpp|include)/' -ExcludeHeaderFilterRegex: 'cuda/core/_include/(dlpack|aoti_shim)\.h' +HeaderFilterRegex: 'cuda[/\\]core[/\\]_(cpp|include)[/\\]' +ExcludeHeaderFilterRegex: 'cuda[/\\]core[/\\]_include[/\\](dlpack|aoti_shim)\.h' diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index 6a7df8f999f..bc92f890f2e 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -14,8 +14,10 @@ import re import subprocess import sys +import sysconfig import tempfile import zipfile +from collections.abc import Sequence from pathlib import Path import Cython as _Cython @@ -705,37 +707,99 @@ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): return wheel_name -def _find_clang_tidy() -> str: - """The clang-tidy executable of the clang-tidy wheel in the build environment. +def _find_clang_tidy() -> str | None: + """The clang-tidy executable of the build environment. - The wheel is a build-system requirement in pyproject.toml. + Isolated builds install the clang-tidy wheel that pyproject.toml requires. + It is located through the wheel's ``clang_tidy`` package. The pixi build + uses the ``clang-tools`` conda package, which puts the executable in a + different location. - It is located through the wheel's ``clang_tidy`` package, never PATH, so a - system clang-tidy cannot be picked up. Raises RuntimeError when the wheel is - not installed. + PATH is never searched, so a system clang-tidy cannot be picked up. + Returns None when neither is installed. """ + exe = f"clang-tidy{sysconfig.get_config_var('EXE')}" + if sys.platform == "win32": + conda_dir = Path("Library") / "bin" + else: + conda_dir = "bin" + candidates = [Path(sys.prefix) / conda_dir / exe] spec = importlib.util.find_spec("clang_tidy") if spec is not None and spec.submodule_search_locations is not None: - exe = "clang-tidy" - if sys.platform == "win32": - exe = "clang-tidy.exe" - for base in spec.submodule_search_locations: - candidate = Path(base) / "data" / "bin" / exe - if candidate.is_file(): - return str(candidate) - raise RuntimeError( - "The clang-tidy build requirement of pyproject.toml is not installed. Isolated builds install it " - "automatically. For other builds, install the pinned version of it." + wheel_candidates = [Path(base) / "data" / "bin" / exe for base in spec.submodule_search_locations] + candidates = wheel_candidates + candidates + for candidate in candidates: + if candidate.is_file(): + return str(candidate) + return None + + +@functools.cache +def _warn_clang_tidy_missing() -> None: + """Warn, once per build, that the clang-tidy checks do not run.""" + banner = "!" * 78 + print( + f"\n{banner}\n" + "WARNING: clang-tidy is not installed in the build environment, so the clang-tidy\n" + "checks of the handwritten C++ sources will NOT run.\n" + "Isolated builds install the clang-tidy wheel automatically. For other builds,\n" + "install the version that pyproject.toml pins, as the wheel or as the conda-forge\n" + "clang-tools package. The wheel does not exist on Windows ARM64.\n" + f"{banner}\n", + file=sys.stderr, + flush=True, ) -def _is_handwritten_cpp(path) -> bool: - """Whether path is a handwritten C++ source, not a Cython-generated one.""" - path = Path(path) - return path.suffix == ".cpp" and path.resolve().is_relative_to(_PACKAGE_DIR / "_cpp") +def _get_handwritten_cpp_files(compile_command: Sequence[str]) -> list[Path]: + """The handwritten C++ sources, not Cython-generated ones, that a compiler command line compiles.""" + handwritten_dir = (_PACKAGE_DIR / "_cpp").resolve() + files: list[Path] = [] + for arg in compile_command: + # MSVC passes each C++ source as /Tp, which is removed from the result. + path = Path(arg.removeprefix("/Tp")) + if path.suffix == ".cpp" and path.resolve().is_relative_to(handwritten_dir): + files.append(path) + return files + +@functools.cache +def _get_implicit_include_dirs(compiler: str) -> list[str]: + """The include directories that a GCC-style compiler searches without being told. + + They are built into the compiler, so they are not on the command line that + setuptools runs, and clang-tidy cannot find them for a toolchain such as the + conda cross-compiler, whose layout it does not know. Examples are the C++ + standard library and the compiler's own limits.h. They are returned in + search order. Raises RuntimeError when the compiler does not list them. + """ + # MSVC's include directories come from the INCLUDE environment variable, + # which clang-tidy reads in its MSVC mode. Elsewhere clang-tidy needs the + # include directories that the compiler has built in. + if sys.platform == "win32": + return [] -def check_with_clang_tidy(compile_command) -> None: + result = subprocess.run( # noqa: S603 + [compiler, "-E", "-x", "c++", "-", "-v"], + input="", + capture_output=True, + text=True, + ) + dirs: list[str] = [] + in_list = False + for line in result.stderr.splitlines(): + if line.startswith("#include <...> search starts here:"): + in_list = True + elif line.startswith("End of search list."): + break + elif in_list and not line.endswith("(framework directory)"): + dirs.append(line.strip()) + if result.returncode != 0 or not dirs: + raise RuntimeError(f"Failed to query the implicit include directories of {compiler}:\n{result.stderr}") + return dirs + + +def check_with_clang_tidy(compile_command: Sequence[str]) -> None: """Run clang-tidy on the handwritten .cpp that a compiler command line compiles. ``compile_command`` is the exact command that setuptools is about to spawn, @@ -743,14 +807,24 @@ def check_with_clang_tidy(compile_command) -> None: flags as the build. Commands that do not compile a handwritten source are ignored. The configuration is cuda_core/.clang-tidy, which turns every finding into an error. Raises RuntimeError with the findings. + + When clang-tidy is not installed, the check is skipped with a prominent + warning. """ - sources = [arg for arg in compile_command if _is_handwritten_cpp(arg)] + sources = _get_handwritten_cpp_files(compile_command) if not sources: return (source,) = sources + clang_tidy = _find_clang_tidy() + if clang_tidy is None: + _warn_clang_tidy_missing() + return config = Path(__file__).resolve().parent / ".clang-tidy" + implicit_includes: list[str] = [] + for include_dir in _get_implicit_include_dirs(compile_command[0]): + implicit_includes.extend(["-isystem", include_dir]) result = subprocess.run( # noqa: S603 - [_find_clang_tidy(), f"--config-file={config}", "--quiet", source, "--", *compile_command], + [clang_tidy, f"--config-file={config}", "--quiet", source, "--", *compile_command, *implicit_includes], capture_output=True, text=True, ) diff --git a/cuda_core/cuda/core/_cpp/rt/py.hpp b/cuda_core/cuda/core/_cpp/rt/py.hpp index cc8c2f61226..17afa1c4a1b 100644 --- a/cuda_core/cuda/core/_cpp/rt/py.hpp +++ b/cuda_core/cuda/core/_cpp/rt/py.hpp @@ -11,7 +11,7 @@ namespace cuda_core::rt { -#if PY_VERSION_HEX < 0x030D0000 +#if PY_VERSION_HEX < 0x030D0000 && defined(Py_LIMITED_API) extern "C" int _Py_IsFinalizing(void); #endif diff --git a/cuda_core/pixi.toml b/cuda_core/pixi.toml index 3d490c05cdf..e2a91ae84ca 100644 --- a/cuda_core/pixi.toml +++ b/cuda_core/pixi.toml @@ -270,6 +270,7 @@ cython = ">=3.2.5,<3.3" cuda-nvrtc-dev = "*" cuda-bindings = "*" dlpack = "*" +clang-tools = "==22.1.8" [package.target.linux-64.host-dependencies] cuda-cudart-dev_linux-64 = "*" diff --git a/cuda_core/pyproject.toml b/cuda_core/pyproject.toml index 705da894c5c..48ff6df460b 100644 --- a/cuda_core/pyproject.toml +++ b/cuda_core/pyproject.toml @@ -8,7 +8,7 @@ requires = [ "setuptools-scm[simple]>=8,!=10.1", "Cython>=3.2.5,<3.3", "cuda-pathfinder>=1.5", - "clang-tidy==22.1.8", + "clang-tidy==22.1.8; sys_platform != 'win32' or platform_machine != 'ARM64'", ] build-backend = "build_hooks" backend-path = ["."] From d315dce8ded03a9907b8f3424857bc684d0985e7 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 10:40:03 -0400 Subject: [PATCH 03/12] Fix small formatting nit --- cuda_core/cuda/core/_cpp/rt/memory.cpp | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cuda_core/cuda/core/_cpp/rt/memory.cpp b/cuda_core/cuda/core/_cpp/rt/memory.cpp index d6b4dd5754f..f6e46b2dd93 100644 --- a/cuda_core/cuda/core/_cpp/rt/memory.cpp +++ b/cuda_core/cuda/core/_cpp/rt/memory.cpp @@ -626,20 +626,20 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co } // clang-format off - auto box = std::shared_ptr( - new DevicePtrBox{ptr, std::move(ds)}, - [h_pool](DevicePtrBox *b) { - GILReleaseGuard gil; - const DeallocationStream& stream = b->deallocation; - cleanup_in_context( - deallocation_context(stream), "cuMemFreeAsync", handle_bits(b->resource), - [&]() noexcept { - return DRIVER_CALL(cuMemFreeAsync, - b->resource, as_cu(stream.h_stream)); - }); - delete b; - } - ); + auto box = std::shared_ptr( + new DevicePtrBox{ptr, std::move(ds)}, + [h_pool](DevicePtrBox *b) { + GILReleaseGuard gil; + const DeallocationStream& stream = b->deallocation; + cleanup_in_context( + deallocation_context(stream), "cuMemFreeAsync", handle_bits(b->resource), + [&]() noexcept { + return DRIVER_CALL(cuMemFreeAsync, + b->resource, as_cu(stream.h_stream)); + }); + delete b; + } + ); // clang-format on return DevicePtrHandle(box, &box->resource); } From 8a90fda22c1015042a6b6714eb89af0916c7e0c8 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 10:53:07 -0400 Subject: [PATCH 04/12] Further bugfixes --- cuda_core/build_hooks.py | 69 ++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index bc92f890f2e..f1501d7d232 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -707,6 +707,7 @@ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): return wheel_name +@functools.cache def _find_clang_tidy() -> str | None: """The clang-tidy executable of the build environment. @@ -751,20 +752,40 @@ def _warn_clang_tidy_missing() -> None: ) -def _get_handwritten_cpp_files(compile_command: Sequence[str]) -> list[Path]: - """The handwritten C++ sources, not Cython-generated ones, that a compiler command line compiles.""" +def _partition_handwritten_cpp_files(args: Sequence[str]) -> tuple[list[Path], list[str]]: + """Split compiler arguments into the handwritten C++ sources and the other arguments. + + Cython-generated sources are not handwritten, so they stay among the other + arguments. + """ handwritten_dir = (_PACKAGE_DIR / "_cpp").resolve() - files: list[Path] = [] - for arg in compile_command: - # MSVC passes each C++ source as /Tp, which is removed from the result. - path = Path(arg.removeprefix("/Tp")) + sources: list[Path] = [] + other_args: list[str] = [] + for arg in args: + path = Path(arg) if path.suffix == ".cpp" and path.resolve().is_relative_to(handwritten_dir): - files.append(path) - return files + sources.append(path) + else: + other_args.append(arg) + return sources, other_args + + +def _split_compile_command(compile_command: Sequence[str]) -> tuple[tuple[str, ...], list[str]]: + """Split a compiler command line into the compiler and its arguments. + + The compiler is the leading words that are not options: a compiler launcher + such as sccache or ccache, if any, and the compiler. MSVC options start with + "/", like paths, so there the compiler is the first word only. + """ + n = 1 + if sys.platform != "win32": + while n < len(compile_command) and not compile_command[n].startswith("-"): + n += 1 + return tuple(compile_command[:n]), list(compile_command[n:]) @functools.cache -def _get_implicit_include_dirs(compiler: str) -> list[str]: +def _get_implicit_include_dirs(compiler: tuple[str, ...]) -> list[str]: """The include directories that a GCC-style compiler searches without being told. They are built into the compiler, so they are not on the command line that @@ -780,7 +801,7 @@ def _get_implicit_include_dirs(compiler: str) -> list[str]: return [] result = subprocess.run( # noqa: S603 - [compiler, "-E", "-x", "c++", "-", "-v"], + [*compiler, "-E", "-x", "c++", "-", "-v"], input="", capture_output=True, text=True, @@ -795,7 +816,9 @@ def _get_implicit_include_dirs(compiler: str) -> list[str]: elif in_list and not line.endswith("(framework directory)"): dirs.append(line.strip()) if result.returncode != 0 or not dirs: - raise RuntimeError(f"Failed to query the implicit include directories of {compiler}:\n{result.stderr}") + raise RuntimeError( + f"Failed to query the implicit include directories of {' '.join(compiler)}:\n{result.stderr}" + ) return dirs @@ -809,22 +832,28 @@ def check_with_clang_tidy(compile_command: Sequence[str]) -> None: finding into an error. Raises RuntimeError with the findings. When clang-tidy is not installed, the check is skipped with a prominent - warning. + warning, whether or not the command compiles a handwritten source. """ - sources = _get_handwritten_cpp_files(compile_command) - if not sources: - return - (source,) = sources clang_tidy = _find_clang_tidy() if clang_tidy is None: _warn_clang_tidy_missing() return + compiler, args = _split_compile_command(compile_command) + # MSVC passes each C++ source as /Tp. + args = [arg.removeprefix("/Tp") for arg in args] + sources, args = _partition_handwritten_cpp_files(args) + if not sources: + return + (source,) = sources config = Path(__file__).resolve().parent / ".clang-tidy" - implicit_includes: list[str] = [] - for include_dir in _get_implicit_include_dirs(compile_command[0]): - implicit_includes.extend(["-isystem", include_dir]) + if sys.platform == "win32": + # Without this clang-tidy reads the MSVC options, such as /std:c++17, + # as file names. + args.insert(0, "--driver-mode=cl") + for include_dir in _get_implicit_include_dirs(compiler): + args.extend(["-isystem", include_dir]) result = subprocess.run( # noqa: S603 - [clang_tidy, f"--config-file={config}", "--quiet", source, "--", *compile_command, *implicit_includes], + [clang_tidy, f"--config-file={config}", "--quiet", source, "--", *args], capture_output=True, text=True, ) From 021937f6c405066172ce7fb28a190a76b42e57e4 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 11:43:24 -0400 Subject: [PATCH 05/12] More bugfixes --- cuda_core/build_hooks.py | 4 +++- cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index f1501d7d232..9b6ce3adf88 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -848,8 +848,10 @@ def check_with_clang_tidy(compile_command: Sequence[str]) -> None: config = Path(__file__).resolve().parent / ".clang-tidy" if sys.platform == "win32": # Without this clang-tidy reads the MSVC options, such as /std:c++17, - # as file names. + # as file names. It does not use some of them, such as /c and /GL, and + # reports each unused option as an error. args.insert(0, "--driver-mode=cl") + args.append("-Wno-unused-command-line-argument") for include_dir in _get_implicit_include_dirs(compiler): args.extend(["-isystem", include_dir]) result = subprocess.run( # noqa: S603 diff --git a/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp b/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp index c4ca537373a..7837aa4d9d1 100644 --- a/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp +++ b/cuda_core/cuda/core/_cpp/rt/py_driver_fns.cpp @@ -108,7 +108,9 @@ take_python_error(char *buf, std::size_t size, bool *interrupted) noexcept #if PY_VERSION_HEX >= 0x030C0000 PyObject *exc = PyErr_GetRaisedException(); #else - PyObject *type, *value, *traceback; + PyObject *type = nullptr; + PyObject *value = nullptr; + PyObject *traceback = nullptr; PyErr_Fetch(&type, &value, &traceback); PyErr_NormalizeException(&type, &value, &traceback); PyObject *exc = value; From c4bc3593a54ab5b0facccfc71bd60027540adce2 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 12:01:42 -0400 Subject: [PATCH 06/12] Display all errors before quitting --- cuda_core/build_hooks.py | 8 ++++++-- cuda_core/setup.py | 12 +++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index 9b6ce3adf88..01acef99e05 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -822,6 +822,10 @@ def _get_implicit_include_dirs(compiler: tuple[str, ...]) -> list[str]: return dirs +class ClangTidyError(RuntimeError): + """clang-tidy reported errors in a handwritten source.""" + + def check_with_clang_tidy(compile_command: Sequence[str]) -> None: """Run clang-tidy on the handwritten .cpp that a compiler command line compiles. @@ -829,7 +833,7 @@ def check_with_clang_tidy(compile_command: Sequence[str]) -> None: so clang-tidy sees the same compiler, include directories, macros and flags as the build. Commands that do not compile a handwritten source are ignored. The configuration is cuda_core/.clang-tidy, which turns every - finding into an error. Raises RuntimeError with the findings. + finding into an error. Raises ClangTidyError with the findings. When clang-tidy is not installed, the check is skipped with a prominent warning, whether or not the command compiles a handwritten source. @@ -860,7 +864,7 @@ def check_with_clang_tidy(compile_command: Sequence[str]) -> None: text=True, ) if result.returncode != 0: - raise RuntimeError(f"clang-tidy reported errors in {source}:\n{result.stdout}{result.stderr}") + raise ClangTidyError(f"clang-tidy reported errors in {source}:\n{result.stdout}{result.stderr}") def _get_cuda_bindings_require(): diff --git a/cuda_core/setup.py b/cuda_core/setup.py index af8b5b193cf..49f18139e51 100644 --- a/cuda_core/setup.py +++ b/cuda_core/setup.py @@ -94,15 +94,23 @@ def _clang_tidy_before_compile(self): clang-tidy gets the exact command line of the build. That method is call() in current setuptools, where spawn() is deprecated and delegates to it, and spawn() before. Only sources that are rebuilt are checked. + + A source with findings does not stop the build. Every source is checked + and compiled, and the findings of all of them are raised together once + the build is over, so that one run reports everything to fix. """ compiler = self.compiler method = "spawn" if hasattr(compiler, "call"): method = "call" stock = getattr(compiler, method) + failures = [] def run(cmd, *args, **kwargs): - build_hooks.check_with_clang_tidy(cmd) + try: + build_hooks.check_with_clang_tidy(cmd) + except build_hooks.ClangTidyError as e: + failures.append(str(e)) return stock(cmd, *args, **kwargs) setattr(compiler, method, run) @@ -110,6 +118,8 @@ def run(cmd, *args, **kwargs): yield finally: setattr(compiler, method, stock) + if failures: + raise build_hooks.ClangTidyError("\n\n".join(failures)) @contextlib.contextmanager def _parallel_source_compilation(self): From 87fb95ab88bc0b39cad56228b3e343661e0146d2 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 12:15:29 -0400 Subject: [PATCH 07/12] Fix Windows clang-tidy issue --- cuda_core/cuda/core/_cpp/rt/internal.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cuda_core/cuda/core/_cpp/rt/internal.hpp b/cuda_core/cuda/core/_cpp/rt/internal.hpp index 56c935b0103..c44ee9408e3 100644 --- a/cuda_core/cuda/core/_cpp/rt/internal.hpp +++ b/cuda_core/cuda/core/_cpp/rt/internal.hpp @@ -238,7 +238,10 @@ class HandleRegistry { return h; } + // MSVC's std::unordered_map allocates in its default constructor, so an + // allocation failure here terminates. Running out of memory is fatal either way. MapType + // NOLINTNEXTLINE(bugprone-exception-escape) drain() noexcept { std::scoped_lock lock(mutex_); From 03c6b31cbc574aff5277d46160794ea8a59f67be Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 29 Oct 2026 12:15:41 -0400 Subject: [PATCH 08/12] Remove slop comment --- cuda_core/MANIFEST.in | 1 - 1 file changed, 1 deletion(-) diff --git a/cuda_core/MANIFEST.in b/cuda_core/MANIFEST.in index 91e3aa3bc4b..b723f52348b 100644 --- a/cuda_core/MANIFEST.in +++ b/cuda_core/MANIFEST.in @@ -6,7 +6,6 @@ recursive-include cuda/core *.pyx *.pxd *.pxi *.pyi recursive-include cuda/core/_cpp *.cpp *.h *.hpp recursive-include cuda/core/_include *.h *.hpp *.def include build_hooks.py -# configuration of the clang-tidy run in the build include .clang-tidy include cuda/core/py.typed include NOTICE From febf1c7bde50bfbdb62742e735aa023550b1003b Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 3 Nov 2026 14:51:47 -0500 Subject: [PATCH 09/12] Provide a way to opt-out of clang-tidy --- cuda_core/AGENTS.md | 2 ++ cuda_core/README.md | 4 ++++ cuda_core/build_hooks.py | 5 +++++ cuda_core/tests/test_build_hooks.py | 16 ++++++++++++++++ 4 files changed, 27 insertions(+) diff --git a/cuda_core/AGENTS.md b/cuda_core/AGENTS.md index 2febc5374ed..a516458796d 100644 --- a/cuda_core/AGENTS.md +++ b/cuda_core/AGENTS.md @@ -117,6 +117,8 @@ accepted. Do not add fallback logic for it. The order is: - `CUDA_PYTHON_PARALLEL_LEVEL` - `CUDA_PYTHON_COVERAGE` - `CUDA_PYTHON_WERROR` (compiler warnings become errors; CI wheel builds set it) + - `CUDA_PYTHON_SKIP_CLANG_TIDY` (set to `1` to skip the clang-tidy check of + the handwritten C++ sources during the build) ## Editing guidance diff --git a/cuda_core/README.md b/cuda_core/README.md index 74b2b6638f7..42e9aab5ed1 100644 --- a/cuda_core/README.md +++ b/cuda_core/README.md @@ -24,6 +24,10 @@ To run these tests: * `python -m pytest tests/` with editable installations * `pytest tests/` with installed packages +### Building from source + +Builds run clang-tidy on the handwritten C++ sources (`cuda/core/_cpp/`) and fail on any finding. To skip this check, for example when the installed clang-tidy differs from the pinned version, set `CUDA_PYTHON_SKIP_CLANG_TIDY=1` in the build environment. + ### Cython Unit Tests Cython tests are located in `tests/cython` and need to be built. These builds have the same CUDA Toolkit header requirements as [those of cuda.bindings](https://nvidia.github.io/cuda-python/cuda-bindings/latest/install.html#requirements) where the major.minor version must match `cuda.bindings`. To build them: diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index 01acef99e05..dc24128d945 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -62,6 +62,8 @@ def __getattr__(name): # compilers we do not control. Meant for optimized builds: a debug build # (-O0) trips the _FORTIFY_SOURCE "#warning" on glibc toolchains. WARNINGS_AS_ERRORS = bool(int(os.environ.get("CUDA_PYTHON_WERROR", "0"))) +# CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the clang-tidy check of the handwritten +# C++ sources (see check_with_clang_tidy). It is read when the check runs. def _import_cuda_bindings(): @@ -837,7 +839,10 @@ def check_with_clang_tidy(compile_command: Sequence[str]) -> None: When clang-tidy is not installed, the check is skipped with a prominent warning, whether or not the command compiles a handwritten source. + Setting CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check silently. """ + if bool(int(os.environ.get("CUDA_PYTHON_SKIP_CLANG_TIDY", "0"))): + return clang_tidy = _find_clang_tidy() if clang_tidy is None: _warn_clang_tidy_missing() diff --git a/cuda_core/tests/test_build_hooks.py b/cuda_core/tests/test_build_hooks.py index b4e3361db5e..e4ac00e7824 100644 --- a/cuda_core/tests/test_build_hooks.py +++ b/cuda_core/tests/test_build_hooks.py @@ -1100,3 +1100,19 @@ class TestCythonAlias(CythonAliasMixin): """`_stable_cython_alias` tests for cuda.core.""" build_hooks = build_hooks + + +@pytest.mark.agent_authored(model="claude-sonnet-5-5") +def test_clang_tidy_skipped_by_env_var(monkeypatch): + """CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check before clang-tidy is even looked up.""" + lookups = [] + monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: lookups.append(1)) + monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "1") + build_hooks.check_with_clang_tidy(["g++", "-c", "x.cpp"]) + assert lookups == [] + + monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "0") + build_hooks._warn_clang_tidy_missing.cache_clear() + build_hooks.check_with_clang_tidy(["g++", "-c", "x.cpp"]) + assert lookups == [1] + build_hooks._warn_clang_tidy_missing.cache_clear() From 3ec5018009ff13db77ee4a0fd9af3377afff8be1 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 3 Nov 2026 14:53:48 -0500 Subject: [PATCH 10/12] Improve buffer sizes --- cuda_core/cuda/core/_cpp/rt/error.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cuda_core/cuda/core/_cpp/rt/error.cpp b/cuda_core/cuda/core/_cpp/rt/error.cpp index 97f2877bb69..14567c84346 100644 --- a/cuda_core/cuda/core/_cpp/rt/error.cpp +++ b/cuda_core/cuda/core/_cpp/rt/error.cpp @@ -30,6 +30,10 @@ namespace { thread_local char last_error_detail[kErrorMessageSize] = {0}; thread_local char taken_error_detail[kErrorMessageSize] = {0}; thread_local CUresult last_error_detail_status = CUDA_SUCCESS; + +// The restore-failure cause is a short fixed phrase plus an error name. Keeping +// it well below kErrorMessageSize lets the compiler prove the final message fits. +constexpr std::size_t kCauseSize = 128; } // namespace namespace detail { @@ -122,7 +126,7 @@ note_context_not_restored(CUcontext previous, CUresult operation_status, CUresul if (DRIVER_CALL(cuCtxGetCurrent, ¤t) != CUDA_SUCCESS) { current = nullptr; } - char cause[kErrorMessageSize] = {0}; + char cause[kCauseSize] = {0}; if (operation_status != CUDA_SUCCESS) { const char *error_name = nullptr; if (DRIVER_CALL(cuGetErrorName, restore_status, &error_name) == CUDA_SUCCESS) { From 2b7aac6485870ad030a2af0fa614abf6af7374aa Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 3 Nov 2026 15:01:29 -0500 Subject: [PATCH 11/12] Fix some issues found in the PR --- cuda_core/build_hooks.py | 56 ++++++++++++------------ cuda_core/setup.py | 23 ++++++++-- cuda_core/tests/test_build_hooks.py | 67 ++++++++++++++++++++++++----- 3 files changed, 104 insertions(+), 42 deletions(-) diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index dc24128d945..a221b3bac1e 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -737,23 +737,6 @@ def _find_clang_tidy() -> str | None: return None -@functools.cache -def _warn_clang_tidy_missing() -> None: - """Warn, once per build, that the clang-tidy checks do not run.""" - banner = "!" * 78 - print( - f"\n{banner}\n" - "WARNING: clang-tidy is not installed in the build environment, so the clang-tidy\n" - "checks of the handwritten C++ sources will NOT run.\n" - "Isolated builds install the clang-tidy wheel automatically. For other builds,\n" - "install the version that pyproject.toml pins, as the wheel or as the conda-forge\n" - "clang-tools package. The wheel does not exist on Windows ARM64.\n" - f"{banner}\n", - file=sys.stderr, - flush=True, - ) - - def _partition_handwritten_cpp_files(args: Sequence[str]) -> tuple[list[Path], list[str]]: """Split compiler arguments into the handwritten C++ sources and the other arguments. @@ -828,25 +811,42 @@ class ClangTidyError(RuntimeError): """clang-tidy reported errors in a handwritten source.""" -def check_with_clang_tidy(compile_command: Sequence[str]) -> None: +def get_clang_tidy() -> str | None: + """The clang-tidy executable to check the build with, or None to skip the check. + + Call this once, before the compiles start: it warns prominently when + clang-tidy is not installed. Setting CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips + the check silently. + """ + if bool(int(os.environ.get("CUDA_PYTHON_SKIP_CLANG_TIDY", "0"))): + return None + clang_tidy = _find_clang_tidy() + if clang_tidy is None: + banner = "!" * 78 + print( + f"\n{banner}\n" + "WARNING: clang-tidy is not installed in the build environment, so the clang-tidy\n" + "checks of the handwritten C++ sources will NOT run.\n" + "Isolated builds install the clang-tidy wheel automatically. For other builds,\n" + "install the version that pyproject.toml pins, as the wheel or as the conda-forge\n" + "clang-tools package. The wheel does not exist on Windows ARM64.\n" + f"{banner}\n", + file=sys.stderr, + flush=True, + ) + return clang_tidy + + +def check_with_clang_tidy(clang_tidy: str, compile_command: Sequence[str]) -> None: """Run clang-tidy on the handwritten .cpp that a compiler command line compiles. + ``clang_tidy`` is the executable from get_clang_tidy(). ``compile_command`` is the exact command that setuptools is about to spawn, so clang-tidy sees the same compiler, include directories, macros and flags as the build. Commands that do not compile a handwritten source are ignored. The configuration is cuda_core/.clang-tidy, which turns every finding into an error. Raises ClangTidyError with the findings. - - When clang-tidy is not installed, the check is skipped with a prominent - warning, whether or not the command compiles a handwritten source. - Setting CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check silently. """ - if bool(int(os.environ.get("CUDA_PYTHON_SKIP_CLANG_TIDY", "0"))): - return - clang_tidy = _find_clang_tidy() - if clang_tidy is None: - _warn_clang_tidy_missing() - return compiler, args = _split_compile_command(compile_command) # MSVC passes each C++ source as /Tp. args = [arg.removeprefix("/Tp") for arg in args] diff --git a/cuda_core/setup.py b/cuda_core/setup.py index 49f18139e51..0dd755ac07d 100644 --- a/cuda_core/setup.py +++ b/cuda_core/setup.py @@ -97,8 +97,16 @@ def _clang_tidy_before_compile(self): A source with findings does not stop the build. Every source is checked and compiled, and the findings of all of them are raised together once - the build is over, so that one run reports everything to fix. + the build is over, so that one run reports everything to fix. If the + build itself fails, that error propagates and the findings are attached + to it as a note. """ + # Looked up here, on the main thread, because the sources compile on + # several threads and a missing clang-tidy must be reported only once. + clang_tidy = build_hooks.get_clang_tidy() + if clang_tidy is None: + yield + return compiler = self.compiler method = "spawn" if hasattr(compiler, "call"): @@ -108,7 +116,7 @@ def _clang_tidy_before_compile(self): def run(cmd, *args, **kwargs): try: - build_hooks.check_with_clang_tidy(cmd) + build_hooks.check_with_clang_tidy(clang_tidy, cmd) except build_hooks.ClangTidyError as e: failures.append(str(e)) return stock(cmd, *args, **kwargs) @@ -116,10 +124,17 @@ def run(cmd, *args, **kwargs): setattr(compiler, method, run) try: yield - finally: - setattr(compiler, method, stock) + except BaseException as e: + # The compile error is the real failure. Do not replace it with the + # findings; attach them so they are still reported. + if failures: + e.add_note("\n\n".join(failures)) + raise + else: if failures: raise build_hooks.ClangTidyError("\n\n".join(failures)) + finally: + setattr(compiler, method, stock) @contextlib.contextmanager def _parallel_source_compilation(self): diff --git a/cuda_core/tests/test_build_hooks.py b/cuda_core/tests/test_build_hooks.py index e4ac00e7824..26468303f99 100644 --- a/cuda_core/tests/test_build_hooks.py +++ b/cuda_core/tests/test_build_hooks.py @@ -1103,16 +1103,63 @@ class TestCythonAlias(CythonAliasMixin): @pytest.mark.agent_authored(model="claude-sonnet-5-5") -def test_clang_tidy_skipped_by_env_var(monkeypatch): - """CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check before clang-tidy is even looked up.""" - lookups = [] - monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: lookups.append(1)) +def test_get_clang_tidy_skipped_by_env_var(monkeypatch, capsys): + """CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check silently, even when clang-tidy is missing.""" + monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: None) monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "1") - build_hooks.check_with_clang_tidy(["g++", "-c", "x.cpp"]) - assert lookups == [] + assert build_hooks.get_clang_tidy() is None + assert capsys.readouterr().err == "" + +@pytest.mark.agent_authored(model="claude-sonnet-5-5") +def test_get_clang_tidy_warns_when_missing(monkeypatch, capsys): + monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: None) monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "0") - build_hooks._warn_clang_tidy_missing.cache_clear() - build_hooks.check_with_clang_tidy(["g++", "-c", "x.cpp"]) - assert lookups == [1] - build_hooks._warn_clang_tidy_missing.cache_clear() + assert build_hooks.get_clang_tidy() is None + assert "WARNING: clang-tidy is not installed" in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-sonnet-5-5") +def test_get_clang_tidy_returns_executable(monkeypatch): + monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: "/opt/clang-tidy") + monkeypatch.delenv("CUDA_PYTHON_SKIP_CLANG_TIDY", raising=False) + assert build_hooks.get_clang_tidy() == "/opt/clang-tidy" + + +class TestClangTidyBeforeCompile: + """_clang_tidy_before_compile: findings are raised after a good build, noted on a failed one.""" + + @staticmethod + def _run(monkeypatch, body): + from setuptools.dist import Distribution + + setup_py = _load_setup_py(monkeypatch) + + def fake_check(clang_tidy, cmd): + raise build_hooks.ClangTidyError("finding in x.cpp") + + monkeypatch.setattr(build_hooks, "get_clang_tidy", lambda: "clang-tidy") + monkeypatch.setattr(build_hooks, "check_with_clang_tidy", fake_check) + cmd = setup_py.build_ext(Distribution({"name": "cuda-core", "version": "0"})) + + def stock_spawn(cmd, *args, **kwargs): + del cmd, args, kwargs + + cmd.compiler = types.SimpleNamespace(spawn=stock_spawn) + with cmd._clang_tidy_before_compile(): + body(cmd.compiler) + + @pytest.mark.agent_authored(model="claude-sonnet-5-5") + def test_findings_raised_when_build_succeeds(self, monkeypatch): + with pytest.raises(build_hooks.ClangTidyError, match="finding in x.cpp"): + self._run(monkeypatch, lambda compiler: compiler.spawn(["g++", "x.cpp"])) + + @pytest.mark.agent_authored(model="claude-sonnet-5-5") + def test_compile_error_is_not_replaced(self, monkeypatch): + def body(compiler): + compiler.spawn(["g++", "x.cpp"]) + raise OSError("compile failed") + + with pytest.raises(OSError, match="compile failed") as info: + self._run(monkeypatch, body) + assert any("finding in x.cpp" in note for note in info.value.__notes__) From 35ec114569a8dd807b4181516bf04d3e2e7ff525 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 3 Nov 2026 15:04:57 -0500 Subject: [PATCH 12/12] Inline find_clang_tidy --- cuda_core/build_hooks.py | 50 ++++++++++++----------------- cuda_core/tests/test_build_hooks.py | 26 +++++++++++---- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/cuda_core/build_hooks.py b/cuda_core/build_hooks.py index a221b3bac1e..5a7b0f139a7 100644 --- a/cuda_core/build_hooks.py +++ b/cuda_core/build_hooks.py @@ -709,34 +709,6 @@ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): return wheel_name -@functools.cache -def _find_clang_tidy() -> str | None: - """The clang-tidy executable of the build environment. - - Isolated builds install the clang-tidy wheel that pyproject.toml requires. - It is located through the wheel's ``clang_tidy`` package. The pixi build - uses the ``clang-tools`` conda package, which puts the executable in a - different location. - - PATH is never searched, so a system clang-tidy cannot be picked up. - Returns None when neither is installed. - """ - exe = f"clang-tidy{sysconfig.get_config_var('EXE')}" - if sys.platform == "win32": - conda_dir = Path("Library") / "bin" - else: - conda_dir = "bin" - candidates = [Path(sys.prefix) / conda_dir / exe] - spec = importlib.util.find_spec("clang_tidy") - if spec is not None and spec.submodule_search_locations is not None: - wheel_candidates = [Path(base) / "data" / "bin" / exe for base in spec.submodule_search_locations] - candidates = wheel_candidates + candidates - for candidate in candidates: - if candidate.is_file(): - return str(candidate) - return None - - def _partition_handwritten_cpp_files(args: Sequence[str]) -> tuple[list[Path], list[str]]: """Split compiler arguments into the handwritten C++ sources and the other arguments. @@ -817,10 +789,30 @@ def get_clang_tidy() -> str | None: Call this once, before the compiles start: it warns prominently when clang-tidy is not installed. Setting CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check silently. + + Isolated builds install the clang-tidy wheel that pyproject.toml requires. + It is located through the wheel's ``clang_tidy`` package. The pixi build + uses the ``clang-tools`` conda package, which puts the executable in a + different location. PATH is never searched, so a system clang-tidy cannot + be picked up. """ if bool(int(os.environ.get("CUDA_PYTHON_SKIP_CLANG_TIDY", "0"))): return None - clang_tidy = _find_clang_tidy() + exe = f"clang-tidy{sysconfig.get_config_var('EXE')}" + if sys.platform == "win32": + conda_dir = Path("Library") / "bin" + else: + conda_dir = "bin" + candidates = [Path(sys.prefix) / conda_dir / exe] + spec = importlib.util.find_spec("clang_tidy") + if spec is not None and spec.submodule_search_locations is not None: + wheel_candidates = [Path(base) / "data" / "bin" / exe for base in spec.submodule_search_locations] + candidates = wheel_candidates + candidates + clang_tidy = None + for candidate in candidates: + if candidate.is_file(): + clang_tidy = str(candidate) + break if clang_tidy is None: banner = "!" * 78 print( diff --git a/cuda_core/tests/test_build_hooks.py b/cuda_core/tests/test_build_hooks.py index 26468303f99..9bb27716dde 100644 --- a/cuda_core/tests/test_build_hooks.py +++ b/cuda_core/tests/test_build_hooks.py @@ -1103,27 +1103,39 @@ class TestCythonAlias(CythonAliasMixin): @pytest.mark.agent_authored(model="claude-sonnet-5-5") -def test_get_clang_tidy_skipped_by_env_var(monkeypatch, capsys): +def test_get_clang_tidy_skipped_by_env_var(monkeypatch, capsys, tmp_path): """CUDA_PYTHON_SKIP_CLANG_TIDY=1 skips the check silently, even when clang-tidy is missing.""" - monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: None) + monkeypatch.setattr(sys, "prefix", str(tmp_path)) + monkeypatch.setattr(importlib.util, "find_spec", lambda _name: None) monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "1") assert build_hooks.get_clang_tidy() is None assert capsys.readouterr().err == "" @pytest.mark.agent_authored(model="claude-sonnet-5-5") -def test_get_clang_tidy_warns_when_missing(monkeypatch, capsys): - monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: None) +def test_get_clang_tidy_warns_when_missing(monkeypatch, capsys, tmp_path): + monkeypatch.setattr(sys, "prefix", str(tmp_path)) + monkeypatch.setattr(importlib.util, "find_spec", lambda _name: None) monkeypatch.setenv("CUDA_PYTHON_SKIP_CLANG_TIDY", "0") assert build_hooks.get_clang_tidy() is None assert "WARNING: clang-tidy is not installed" in capsys.readouterr().err @pytest.mark.agent_authored(model="claude-sonnet-5-5") -def test_get_clang_tidy_returns_executable(monkeypatch): - monkeypatch.setattr(build_hooks, "_find_clang_tidy", lambda: "/opt/clang-tidy") +def test_get_clang_tidy_finds_executable_in_prefix(monkeypatch, tmp_path): + import sysconfig + + if sys.platform == "win32": + bin_dir = tmp_path / "Library" / "bin" + else: + bin_dir = tmp_path / "bin" + bin_dir.mkdir(parents=True) + exe = bin_dir / f"clang-tidy{sysconfig.get_config_var('EXE')}" + exe.write_text("") + monkeypatch.setattr(sys, "prefix", str(tmp_path)) + monkeypatch.setattr(importlib.util, "find_spec", lambda _name: None) monkeypatch.delenv("CUDA_PYTHON_SKIP_CLANG_TIDY", raising=False) - assert build_hooks.get_clang_tidy() == "/opt/clang-tidy" + assert build_hooks.get_clang_tidy() == str(exe) class TestClangTidyBeforeCompile: