Do not use Unsafe type

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

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 <jakobnybonissen@gmail.com>"]
 
 [weakdeps]

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.

src/basic.jl

@@ -6,9 +6,14 @@ 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
+# 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()
 
@@ -31,10 +36,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 +57,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,50 +97,50 @@ 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
 Base.@propagate_inbounds Base.view(v::MemoryView, idx::AbstractUnitRange) = v[idx]
 
 # 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}

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

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)

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