From 3d829a9e03675188410de561e5a929208b4b6b62 Mon Sep 17 00:00:00 2001 From: Eyal Rozenberg Date: Sat, 27 Jul 2024 00:02:44 +0300 Subject: [PATCH] Fixes #662: `unique_span` "hardening" and tweaks: * Can no longer construct a `unique_span` with another `unique_span` having a different deleter (via the implicit conversion into a span) * Can no longer construct a `unique_span` from an untyped region; make a span and use that. We thus also avoid undesirable constructions via implicit conversions to regions * Comment and spacing tweaks --- src/cuda/api/detail/unique_span.hpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/cuda/api/detail/unique_span.hpp b/src/cuda/api/detail/unique_span.hpp index 746f2e63..549a749c 100644 --- a/src/cuda/api/detail/unique_span.hpp +++ b/src/cuda/api/detail/unique_span.hpp @@ -57,6 +57,14 @@ class unique_span : public ::cuda::span { constexpr unique_span() noexcept = default; + // Disable copy construction - as this class never allocates; + unique_span(const unique_span&) = delete; + // ... and also match other kinds of unique_span's, which may get converted into + // a span and thus leak memory on construction! + template + unique_span(const unique_span&) = delete; + + /// Take ownership of an existing region or span /// /// These ctors are all explicit to prevent accidentally relinquishing ownership @@ -64,7 +72,6 @@ class unique_span : public ::cuda::span { ///@{ explicit unique_span(span_type span) noexcept : span_type{span} { } explicit unique_span(pointer data, size_type size) noexcept : unique_span{span_type{data, size}} { } - explicit unique_span(memory::region_t region) noexcept : span_type{region.as_span()} { } ///@} // Note: No constructor which also takes a deleter. We do not hold a deleter @@ -77,8 +84,10 @@ class unique_span : public ::cuda::span { * the user is strongly assumed not to use the unique_span after moving from it. */ unique_span(unique_span&& other) noexcept : unique_span{ other.release() } { } - // Disable copy construction - we do not allocate - unique_span(const unique_span&) = delete; + + // Disable move construction from other kinds of unique_spans + template + unique_span(unique_span&&) = delete; // Note: No conversion from "another type" like with ::std::unique_pointer, since // this class is not variant with the element type; and there's not much sense in @@ -127,7 +136,13 @@ class unique_span : public ::cuda::span { } protected: // mutators - /// Release ownership of any stored pointer. + /** + * Release ownership of the stored span + * + * @note This is not marked nodiscard by the same argument as for std::unique_ptr; + * see also @url https://stackoverflow.com/q/60535399/1593077 and + * @url http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0600r1.pdf + */ span_type release() noexcept { span_type released { data(), size() };