From a650e15216ce92df02bb4d3da9029d49f90c6aad Mon Sep 17 00:00:00 2001 From: Jakob Nybo Nissen Date: Tue, 8 Apr 2025 10:58:06 +0200 Subject: [PATCH 1/3] Do not use Unsafe type As pointed out by others, this means the method table for public objects contain a mix of public and private methods. That's bad. Instead, use things with private names to do our dirty unsafe business. This is an API change, so I'll leave it for a bit before merging, in case other breaking stuff comes up. --- Project.toml | 2 +- src/MemoryViews.jl | 24 ++++++------------------ src/basic.jl | 31 +++++++++++++++---------------- src/construction.jl | 6 +++--- src/experimental.jl | 4 ++-- test/runtests.jl | 2 +- 6 files changed, 28 insertions(+), 41 deletions(-) diff --git a/Project.toml b/Project.toml index eadc451..a18dfd2 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "MemoryViews" uuid = "a791c907-b98b-4e44-8f4d-e4c2362c6b2f" -version = "0.3.1" +version = "0.4.0" authors = ["Jakob Nybo Nissen "] [weakdeps] diff --git a/src/MemoryViews.jl b/src/MemoryViews.jl index 1c632f2..33fe6b5 100644 --- a/src/MemoryViews.jl +++ b/src/MemoryViews.jl @@ -3,17 +3,6 @@ module MemoryViews export MemoryView, ImmutableMemoryView, MutableMemoryView, MemoryKind, IsMemory, NotMemory, inner -""" - Unsafe - -Trait object used to dispatch to unsafe methods. -The `MemoryViews.unsafe` instance is the singleton instance of this type. -""" -struct Unsafe end - -"Singleton instance of the trait type `Unsafe`" -const unsafe = Unsafe() - """ Trait struct, only used in the mutability parameter of `MemoryView` """ @@ -78,7 +67,7 @@ struct MemoryView{T, M <: Union{Mutable, Immutable}} <: DenseVector{T} ref::MemoryRef{T} len::Int - function MemoryView{T, M}(::Unsafe, ref::MemoryRef{T}, len::Int) where {T, M} + global function unsafe_new_memoryview(::Type{M}, ref::MemoryRef{T}, len::Int) where {M, T} (M === Mutable || M === Immutable) || error("Parameter M must be Mutable or Immutable") return new{T, M}(ref, len) @@ -90,20 +79,19 @@ const ImmutableMemoryView{T} = MemoryView{T, Immutable} # Mutable mem views can turn into immutable ones, but not vice versa ImmutableMemoryView(x) = ImmutableMemoryView(MemoryView(x)::MemoryView) -function ImmutableMemoryView(x::MutableMemoryView{T}) where {T} - return ImmutableMemoryView{T}(unsafe, x.ref, x.len) +function ImmutableMemoryView(x::MemoryView) + return unsafe_new_memoryview(Immutable, x.ref, x.len) end -ImmutableMemoryView(x::ImmutableMemoryView) = x """ - MutableMemoryView(::Unsafe, x::MemoryView) + unsafe_wrap(MutableMemoryView, x::MemoryView) Convert a memory view into a mutable memory view. Note that it may cause undefined behaviour, if supposedly immutable data is observed to be mutated. """ -function MutableMemoryView(::Unsafe, x::MemoryView{T}) where {T} - return MutableMemoryView{T}(unsafe, x.ref, x.len) +function Base.unsafe_wrap(::Type{MutableMemoryView}, x::MemoryView{T}) where {T} + return unsafe_new_memoryview(Mutable, x.ref, x.len) end # Constructors that allows users to specify eltype explicitly, e.g. diff --git a/src/basic.jl b/src/basic.jl index 41ce971..8e6f831 100644 --- a/src/basic.jl +++ b/src/basic.jl @@ -6,9 +6,7 @@ function Base.setindex!(v::MutableMemoryView{T}, x, i::Int) where {T} return v end -# TODO: This uses the internal `.mem` field of `MemoryRef`, but AFAIK there is no -# API in Base to get the memory from a `MemoryRef` -Base.parent(v::MemoryView) = v.ref.mem +Base.parent(v::MemoryView) = parent(v.ref) Base.size(v::MemoryView) = (v.len,) Base.IndexStyle(::Type{<:MemoryView}) = Base.IndexLinear() @@ -31,10 +29,10 @@ function Base.parentindices(x::MemoryView) end end -function Base.copy(x::MemoryView) +function Base.copy(x::MemoryView{T, M}) where {T, M} isempty(x) && return x newmem = @inbounds x.ref.mem[only(parentindices(x))] - return typeof(x)(unsafe, memoryref(newmem), x.len) + return unsafe_new_memoryview(M, memoryref(newmem), x.len) end function Base.getindex(v::MemoryView, i::Integer) @@ -52,12 +50,13 @@ function Base.similar(::MemoryView{T1, M}, ::Type{T2}, dims::Tuple{Int}) where { end function Base.empty(::MemoryView{T1, M}, ::Type{T2}) where {T1, T2, M} - return MemoryView{T2, M}(unsafe, memoryref(Memory{T2}()), 0) + return unsafe_new_memoryview(M, memoryref(Memory{T2}()), 0) end -Base.empty(T::Type{<:MemoryView{E}}) where {E} = T(unsafe, memoryref(Memory{E}()), 0) +Base.empty(::Type{<:MemoryView{E, M}}) where {E, M} = unsafe_new_memoryview(M, memoryref(Memory{E}()), 0) Base.pointer(x::MemoryView{T}) where {T} = Ptr{T}(pointer(x.ref)) Base.unsafe_convert(::Type{Ptr{T}}, v::MemoryView{T}) where {T} = pointer(v) +Base.cconvert(::Type{<:Ptr{T}}, v::MemoryView{T}) where {T} = v.ref Base.elsize(::Type{<:MemoryView{T}}) where {T} = Base.elsize(Memory{T}) Base.sizeof(x::MemoryView) = Base.elsize(typeof(x)) * length(x) Base.strides(::MemoryView) = (1,) @@ -91,14 +90,14 @@ function Base.mightalias(a::KNOWN_MEM_BACKED, b::MemoryView) return Base.mightalias(ImmutableMemoryView(a), b) end -function Base.getindex(v::MemoryView, idx::AbstractUnitRange) +function Base.getindex(v::MemoryView{T, M}, idx::AbstractUnitRange) where {T, M} # This branch is necessary, because the memoryref can't point out of bounds. # So if the user gives an empty slice that is out of bounds, the boundscheck # may pass, but the memoryref construction will be OOB. - isempty(idx) && return typeof(v)(unsafe, memoryref(v.ref.mem), 0) + isempty(idx) && return unsafe_new_memoryview(M, memoryref(v.ref.mem), 0) @boundscheck checkbounds(v, idx) newref = @inbounds memoryref(v.ref, Int(first(idx))::Int) - return typeof(v)(unsafe, newref, length(idx)) + return unsafe_new_memoryview(M, newref, length(idx)) end Base.getindex(v::MemoryView, ::Colon) = v @@ -106,35 +105,35 @@ Base.@propagate_inbounds Base.view(v::MemoryView, idx::AbstractUnitRange) = v[id # Efficient way to get `mem[1:include_last]`. # include_last must be in 0:length(mem) -function truncate(mem::MemoryView, include_last::Integer) +function truncate(mem::MemoryView{T, M}, include_last::Integer) where {T, M} lst = Int(include_last)::Int @boundscheck if (lst % UInt) > length(mem) % UInt throw(BoundsError(mem, lst)) end - return typeof(mem)(unsafe, mem.ref, lst) + return unsafe_new_memoryview(M, mem.ref, lst) end # Efficient way to get `mem[from:end]`. # From must be in 1:length(mem). -function truncate_start_nonempty(mem::MemoryView, from::Integer) +function truncate_start_nonempty(mem::MemoryView{T, M}, from::Integer) where {T, M} frm = Int(from)::Int @boundscheck if ((frm - 1) % UInt) ≥ length(mem) % UInt throw(BoundsError(mem, frm)) end newref = @inbounds memoryref(mem.ref, frm) - return typeof(mem)(unsafe, newref, length(mem) - frm + 1) + return unsafe_new_memoryview(M, newref, length(mem) - frm + 1) end # Efficient way to get `mem[from:end]`. # From must be in 1:length(mem)+1. -function truncate_start(mem::MemoryView, from::Integer) +function truncate_start(mem::MemoryView{T, M}, from::Integer) where {T, M} frm = Int(from)::Int @boundscheck if ((frm - 1) % UInt) > length(mem) % UInt throw(BoundsError(mem, frm)) end frm == 1 && return mem newref = @inbounds memoryref(mem.ref, frm - (from == length(mem) + 1)) - return typeof(mem)(unsafe, newref, length(mem) - frm + 1) + return unsafe_new_memoryview(M, newref, length(mem) - frm + 1) end function Base.unsafe_copyto!(dst::MutableMemoryView{T}, src::MemoryView{T}) where {T} diff --git a/src/construction.jl b/src/construction.jl index ae9b072..c6dd649 100644 --- a/src/construction.jl +++ b/src/construction.jl @@ -3,8 +3,8 @@ MemoryView(v::MemoryView) = v # Array and Memory MemoryKind(::Type{<:Array{T}}) where {T} = IsMemory(MutableMemoryView{T}) MemoryKind(::Type{<:Memory{T}}) where {T} = IsMemory(MutableMemoryView{T}) -MemoryView(A::Memory{T}) where {T} = MutableMemoryView{T}(unsafe, memoryref(A), length(A)) -MemoryView(A::Array{T}) where {T} = MutableMemoryView{T}(unsafe, Base.cconvert(Ptr, A), length(A)) +MemoryView(A::Memory{T}) where {T} = unsafe_new_memoryview(Mutable, memoryref(A), length(A)) +MemoryView(A::Array{T}) where {T} = unsafe_new_memoryview(Mutable, Base.cconvert(Ptr, A), length(A)) # Strings MemoryView(s::String) = ImmutableMemoryView(unsafe_wrap(Memory{UInt8}, s)) @@ -19,7 +19,7 @@ function MemoryView(s::SubString{String}) memview = MemoryView(parent(s)) isempty(memview) && return memview newref = @inbounds memoryref(memview.ref, s.offset + 1) - return ImmutableMemoryView{UInt8}(unsafe, newref, ncodeunits(s)) + return unsafe_new_memoryview(Immutable, newref, ncodeunits(s)) end # CodeUnits are semantically IsMemory, but only if the underlying string diff --git a/src/experimental.jl b/src/experimental.jl index 849af6e..589e90a 100644 --- a/src/experimental.jl +++ b/src/experimental.jl @@ -103,7 +103,7 @@ julia> split_unaligned(MemoryView(collect(0x01:0x20))[6:13], Val(8)) (UInt8[0x06, 0x07, 0x08], UInt8[0x09, 0x0a, 0x0b, 0x0c, 0x0d]) ``` """ -function split_unaligned(v::MemoryView, ::Val{A}) where {A} +function split_unaligned(v::MemoryView{T, M}, ::Val{A}) where {A, T, M} isbitstype(eltype(v)) || error("Alignment can only be computed for views of bitstypes") A isa Bits || error("Invalid alignment") in(A, (1, 2, 4, 8, 16, 32, 64)) || error("Invalid alignment") @@ -112,7 +112,7 @@ function split_unaligned(v::MemoryView, ::Val{A}) where {A} sz = Base.elsize(v) # Early return here to avoid division by zero: Size sz is statically known, # this will be compiled away - iszero(sz) && return (typeof(v)(unsafe, v.ref, 0), v) + iszero(sz) && return (unsafe_new_memoryview(M, v.ref, 0), v) unaligned_bytes = ((alignment - (UInt(pointer(v)) & mask)) & mask) n_elements = min(length(v), div(unaligned_bytes, sz % UInt) % Int) return @inbounds split_at(v, n_elements + 1) diff --git a/test/runtests.jl b/test/runtests.jl index 1770172..83a572c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -37,7 +37,7 @@ MUT_BACKINGS = Any[ @testset "Unsafe mutability" begin v = [1.0, 2.0, 3.0] m = ImmutableMemoryView(v) - m2 = MutableMemoryView(MemoryViews.unsafe, m) + m2 = unsafe_wrap(MutableMemoryView, m) m2[2] = 5.0 @test v == [1.0, 5.0, 3.0] end From 7465133f120d2eea03ab8f67fe5eaa4806660b58 Mon Sep 17 00:00:00 2001 From: Jakob Nybo Nissen Date: Tue, 8 Apr 2025 11:15:16 +0200 Subject: [PATCH 2/3] Fixup: Make parent work on Julia 1.11 --- src/basic.jl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/basic.jl b/src/basic.jl index 8e6f831..c0af371 100644 --- a/src/basic.jl +++ b/src/basic.jl @@ -6,7 +6,14 @@ function Base.setindex!(v::MutableMemoryView{T}, x, i::Int) where {T} return v end -Base.parent(v::MemoryView) = parent(v.ref) +# The parent method for memoryref was added in 1.12. In versions before that, +# it can be accessed by reaching into internals. +@static if VERSION < v"1.12" + Base.parent(v::MemoryView) = v.ref.mem +else + Base.parent(v::MemoryView) = parent(v.ref) +end + Base.size(v::MemoryView) = (v.len,) Base.IndexStyle(::Type{<:MemoryView}) = Base.IndexLinear() From 46c36525da08b69845c7aced48a678179d9bee18 Mon Sep 17 00:00:00 2001 From: Jakob Nybo Nissen Date: Tue, 8 Apr 2025 11:30:36 +0200 Subject: [PATCH 3/3] Add changelog --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index add15b0..9e0303c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ Any new features, or breaking changes, will be written in this file. Bugfixes, internal refactors, documentation improvements and style changes will not be mentioned here, because they do not impact how the package is to be used. +## 0.4.0 +### Breaking changes +* Removed the `Unsafe` trait type: + - Instead of `MutableMemoryView(::Unsafe, ::MemoryView)`, use + `unsafe_wrap(MutableMemoryView, ::MemoryView)` + - Using the inner constructor `MemoryView{T, M}(::Unsafe, ::MemoryRef{T}, ::Int)` + was never documented API and is now removed. + +* `MemoryView(::SubArray)` now accepts fewer subarray types. However, it is unlikely + that any instance that is now no longer accepted worked previously, so it is + unlikely to be breaking in practice. + ## 0.3.0 ### Breaking changes * Change the bounds checking behaviour of the find* functions to match those of