From 30f1ad15ca840fb76420b6ca393cf80c2553bf77 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Wed, 28 Oct 2026 17:50:51 -0400 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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