cleanup MemoryRef (#54647)

While writing docs in #54642, I became dis-satisfied with how `MemoryRef` currently works and since we are approaching the deadline for changing things around here, made this. Previously, we had the builtin `memoryref` which constructed a `GenericMemoryRef` from a `GenericMemory` or from a `GenericMemoryRef` and an offset. and then we defined constructors `GenericMemoryRef`, `MemoryRef` etc that provided a nicish interface around the intrinsic. The problem with this is that people who want to make a `GenericMemoryRef` don't care what kind of `GenericMemoryRef` they are making. That choice is defined by the `GemericMemory`. As such, I have switched it around so that now the intrinsic is named `memoryrefnew` which frees up `memoryref` to be the function that constructs the appropriate type of `GenericMemoryRef`. This could have been done purely on the Base side, but renaming the intrinsic seems worth it to me since Base/Core use the (new) `memoryref` a lot and it seems like we should make the experience the same for internal and external users rather than making `Base` have to work around a bad name.
JuliaLang · Jun 4, 2024 · fa038d9 · fa038d9
1 parent 13635e1
commit fa038d9
Show file tree

Hide file tree

Showing 16 changed files with 77 additions and 79 deletions.
diff --git a/base/array.jl b/base/array.jl
@@ -216,7 +216,7 @@ isbitsunion(u::Type) = u isa Union && allocatedinline(u)
 function _unsetindex!(A::Array, i::Int)
     @inline
     @boundscheck checkbounds(A, i)
-    @inbounds _unsetindex!(GenericMemoryRef(A.ref, i))
+    @inbounds _unsetindex!(memoryref(A.ref, i))
     return A
 end
 
@@ -239,14 +239,14 @@ function isassigned(a::Array, i::Int...)
     @_noub_if_noinbounds_meta
     @boundscheck checkbounds(Bool, a, i...) || return false
     ii = _sub2ind(size(a), i...)
-    return @inbounds isassigned(memoryref(a.ref, ii, false))
+    return @inbounds isassigned(memoryrefnew(a.ref, ii, false))
 end
 
 function isassigned(a::Vector, i::Int) # slight compiler simplification for the most common case
     @inline
     @_noub_if_noinbounds_meta
     @boundscheck checkbounds(Bool, a, i) || return false
-    return @inbounds isassigned(memoryref(a.ref, i, false))
+    return @inbounds isassigned(memoryrefnew(a.ref, i, false))
 end
 
 
@@ -281,7 +281,7 @@ the same manner as C.
 """
 function unsafe_copyto!(dest::Array, doffs, src::Array, soffs, n)
     n == 0 && return dest
-    unsafe_copyto!(GenericMemoryRef(dest.ref, doffs), GenericMemoryRef(src.ref, soffs), n)
+    unsafe_copyto!(memoryref(dest.ref, doffs), memoryref(src.ref, soffs), n)
     return dest
 end
 
@@ -303,8 +303,8 @@ function _copyto_impl!(dest::Union{Array,Memory}, doffs::Integer, src::Union{Arr
     n > 0 || _throw_argerror("Number of elements to copy must be non-negative.")
     @boundscheck checkbounds(dest, doffs:doffs+n-1)
     @boundscheck checkbounds(src, soffs:soffs+n-1)
-    @inbounds let dest = GenericMemoryRef(dest isa Array ? getfield(dest, :ref) : dest, doffs),
-                  src = GenericMemoryRef(src isa Array ? getfield(src, :ref) : src, soffs)
+    @inbounds let dest = memoryref(dest isa Array ? getfield(dest, :ref) : dest, doffs),
+                  src = memoryref(src isa Array ? getfield(src, :ref) : src, soffs)
         unsafe_copyto!(dest, src, n)
     end
     return dest
@@ -348,7 +348,7 @@ copy
     @_nothrow_meta
     ref = a.ref
     newmem = ccall(:jl_genericmemory_copy_slice, Ref{Memory{T}}, (Any, Ptr{Cvoid}, Int), ref.mem, ref.ptr_or_offset, length(a))
-    return $(Expr(:new, :(typeof(a)), :(Core.memoryref(newmem)), :(a.size)))
+    return $(Expr(:new, :(typeof(a)), :(memoryref(newmem)), :(a.size)))
 end
 
 ## Constructors ##
@@ -964,21 +964,21 @@ function setindex! end
 function setindex!(A::Array{T}, x, i::Int) where {T}
     @_noub_if_noinbounds_meta
     @boundscheck (i - 1)%UInt < length(A)%UInt || throw_boundserror(A, (i,))
-    memoryrefset!(memoryref(A.ref, i, false), x isa T ? x : convert(T,x)::T, :not_atomic, false)
+    memoryrefset!(memoryrefnew(A.ref, i, false), x isa T ? x : convert(T,x)::T, :not_atomic, false)
     return A
 end
 function setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T}
     @inline
     @_noub_if_noinbounds_meta
     @boundscheck checkbounds(A, i1, i2, I...) # generally _to_linear_index requires bounds checking
-    memoryrefset!(memoryref(A.ref, _to_linear_index(A, i1, i2, I...), false), x isa T ? x : convert(T,x)::T, :not_atomic, false)
+    memoryrefset!(memoryrefnew(A.ref, _to_linear_index(A, i1, i2, I...), false), x isa T ? x : convert(T,x)::T, :not_atomic, false)
     return A
 end
 
 __safe_setindex!(A::Vector{Any}, @nospecialize(x), i::Int) = (@inline; @_nothrow_noub_meta;
-    memoryrefset!(memoryref(A.ref, i, false), x, :not_atomic, false); return A)
+    memoryrefset!(memoryrefnew(A.ref, i, false), x, :not_atomic, false); return A)
 __safe_setindex!(A::Vector{T}, x::T, i::Int) where {T} = (@inline; @_nothrow_noub_meta;
-    memoryrefset!(memoryref(A.ref, i, false), x, :not_atomic, false); return A)
+    memoryrefset!(memoryrefnew(A.ref, i, false), x, :not_atomic, false); return A)
 __safe_setindex!(A::Vector{T}, x,    i::Int) where {T} = (@inline;
     __safe_setindex!(A, convert(T, x)::T, i))
 
@@ -1050,7 +1050,7 @@ function _growbeg!(a::Vector, delta::Integer)
     setfield!(a, :size, (newlen,))
     # if offset is far enough advanced to fit data in existing memory without copying
     if delta <= offset - 1
-        setfield!(a, :ref, @inbounds GenericMemoryRef(ref, 1 - delta))
+        setfield!(a, :ref, @inbounds memoryref(ref, 1 - delta))
     else
         @noinline (function()
         memlen = length(mem)
@@ -1075,7 +1075,7 @@ function _growbeg!(a::Vector, delta::Integer)
         if ref !== a.ref
             @noinline throw(ConcurrencyViolationError("Vector can not be resized concurrently"))
         end
-        setfield!(a, :ref, @inbounds GenericMemoryRef(newmem, newoffset))
+        setfield!(a, :ref, @inbounds memoryref(newmem, newoffset))
         end)()
     end
     return
@@ -1114,7 +1114,7 @@ function _growend!(a::Vector, delta::Integer)
             newmem = array_new_memory(mem, newmemlen2)
             newoffset = offset
         end
-        newref = @inbounds GenericMemoryRef(newmem, newoffset)
+        newref = @inbounds memoryref(newmem, newoffset)
         unsafe_copyto!(newref, ref, len)
         if ref !== a.ref
             @noinline throw(ConcurrencyViolationError("Vector can not be resized concurrently"))
@@ -1146,7 +1146,7 @@ function _growat!(a::Vector, i::Integer, delta::Integer)
     prefer_start = i <= div(len, 2)
     # if offset is far enough advanced to fit data in beginning of the memory
     if prefer_start && delta <= offset - 1
-        newref = @inbounds GenericMemoryRef(mem, offset - delta)
+        newref = @inbounds memoryref(mem, offset - delta)
         unsafe_copyto!(newref, ref, i)
         setfield!(a, :ref, newref)
         for j in i:i+delta-1
@@ -1163,7 +1163,7 @@ function _growat!(a::Vector, i::Integer, delta::Integer)
         newmemlen = max(overallocation(memlen), len+2*delta+1)
         newoffset = (newmemlen - newlen) ÷ 2 + 1
         newmem = array_new_memory(mem, newmemlen)
-        newref = @inbounds GenericMemoryRef(newmem, newoffset)
+        newref = @inbounds memoryref(newmem, newoffset)
         unsafe_copyto!(newref, ref, i-1)
         unsafe_copyto!(newmem, newoffset + delta + i - 1, mem, offset + i - 1, len - i + 1)
         setfield!(a, :ref, newref)
@@ -1180,7 +1180,7 @@ function _deletebeg!(a::Vector, delta::Integer)
     end
     newlen = len - delta
     if newlen != 0 # if newlen==0 we could accidentally index past the memory
-        newref = @inbounds GenericMemoryRef(a.ref, delta + 1)
+        newref = @inbounds memoryref(a.ref, delta + 1)
         setfield!(a, :ref, newref)
     end
     setfield!(a, :size, (newlen,))
@@ -1497,16 +1497,16 @@ function sizehint!(a::Vector, sz::Integer; first::Bool=false, shrink::Bool=true)
         end
         newmem = array_new_memory(mem, sz)
         if first
-            newref = GenericMemoryRef(newmem, inc + 1)
+            newref = memoryref(newmem, inc + 1)
         else
-            newref = GenericMemoryRef(newmem)
+            newref = memoryref(newmem)
         end
         unsafe_copyto!(newref, ref, len)
         setfield!(a, :ref, newref)
     elseif first
         _growbeg!(a, inc)
         newref = getfield(a, :ref)
-        newref = GenericMemoryRef(newref, inc + 1)
+        newref = memoryref(newref, inc + 1)
         setfield!(a, :size, (len,)) # undo the size change from _growbeg!
         setfield!(a, :ref, newref) # undo the offset change from _growbeg!
     else # last

diff --git a/base/boot.jl b/base/boot.jl
@@ -534,15 +534,12 @@ const undef = UndefInitializer()
 # empty vector constructor
 (self::Type{GenericMemory{kind,T,addrspace}})() where {T,kind,addrspace} = self(undef, 0)
 
+memoryref(mem::GenericMemory) = memoryrefnew(mem)
+memoryref(mem::GenericMemory, i::Integer) = memoryrefnew(memoryrefnew(mem), Int(i), @_boundscheck)
+memoryref(ref::GenericMemoryRef, i::Integer) = memoryrefnew(ref, Int(i), @_boundscheck)
 GenericMemoryRef(mem::GenericMemory) = memoryref(mem)
-GenericMemoryRef(ref::GenericMemoryRef, i::Integer) = memoryref(ref, Int(i), @_boundscheck)
-GenericMemoryRef(mem::GenericMemory, i::Integer) = memoryref(memoryref(mem), Int(i), @_boundscheck)
-GenericMemoryRef{kind,<:Any,AS}(mem::GenericMemory{kind,<:Any,AS}) where {kind,AS} = memoryref(mem)
-GenericMemoryRef{kind,<:Any,AS}(ref::GenericMemoryRef{kind,<:Any,AS}, i::Integer) where {kind,AS} = memoryref(ref, Int(i), @_boundscheck)
-GenericMemoryRef{kind,<:Any,AS}(mem::GenericMemory{kind,<:Any,AS}, i::Integer) where {kind,AS}  = memoryref(memoryref(mem), Int(i), @_boundscheck)
-GenericMemoryRef{kind,T,AS}(mem::GenericMemory{kind,T,AS}) where {kind,T,AS}  = memoryref(mem)
-GenericMemoryRef{kind,T,AS}(ref::GenericMemoryRef{kind,T,AS}, i::Integer) where {kind,T,AS} = memoryref(ref, Int(i), @_boundscheck)
-GenericMemoryRef{kind,T,AS}(mem::GenericMemory{kind,T,AS}, i::Integer) where {kind,T,AS} = memoryref(memoryref(mem), Int(i), @_boundscheck)
+GenericMemoryRef(mem::GenericMemory, i::Integer) = memoryref(mem, i)
+GenericMemoryRef(mem::GenericMemoryRef, i::Integer) = memoryref(mem, i)
 
 const Memory{T} = GenericMemory{:not_atomic, T, CPU}
 const MemoryRef{T} = GenericMemoryRef{:not_atomic, T, CPU}
@@ -650,12 +647,12 @@ module IR
 export CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, ReturnNode,
     NewvarNode, SSAValue, SlotNumber, Argument,
     PiNode, PhiNode, PhiCNode, UpsilonNode, DebugInfo,
-    Const, PartialStruct, InterConditional, EnterNode
+    Const, PartialStruct, InterConditional, EnterNode, memoryref
 
 using Core: CodeInfo, MethodInstance, CodeInstance, GotoNode, GotoIfNot, ReturnNode,
     NewvarNode, SSAValue, SlotNumber, Argument,
     PiNode, PhiNode, PhiCNode, UpsilonNode, DebugInfo,
-    Const, PartialStruct, InterConditional, EnterNode
+    Const, PartialStruct, InterConditional, EnterNode, memoryref
 
 end # module IR
 

diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl
@@ -666,7 +666,7 @@ function iscall_with_boundscheck(@nospecialize(stmt), sv::PostOptAnalysisState)
     f === nothing && return false
     if f === getfield
         nargs = 4
-    elseif f === memoryref || f === memoryrefget || f === memoryref_isassigned
+    elseif f === memoryrefnew || f === memoryrefget || f === memoryref_isassigned
         nargs = 4
     elseif f === memoryrefset!
         nargs = 5

diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl
@@ -2028,7 +2028,7 @@ end
     hasintersect(widenconst(idx), Int) || return Bottom
     return ref
 end
-add_tfunc(memoryref, 1, 3, memoryref_tfunc, 1)
+add_tfunc(memoryrefnew, 1, 3, memoryref_tfunc, 1)
 
 @nospecs function memoryrefoffset_tfunc(𝕃::AbstractLattice, mem)
     hasintersect(widenconst(mem), GenericMemoryRef) || return Bottom
@@ -2181,7 +2181,7 @@ function _builtin_nothrow(𝕃::AbstractLattice, @nospecialize(f::Builtin), argt
                           @nospecialize(rt))
     ⊑ = partialorder(𝕃)
     na = length(argtypes)
-    if f === memoryref
+    if f === memoryrefnew
         return memoryref_builtin_common_nothrow(argtypes)
     elseif f === memoryrefoffset
         length(argtypes) == 1 || return false
@@ -2297,7 +2297,7 @@ const _EFFECT_FREE_BUILTINS = [
     isa,
     UnionAll,
     getfield,
-    memoryref,
+    memoryrefnew,
     memoryrefoffset,
     memoryrefget,
     memoryref_isassigned,
@@ -2332,7 +2332,7 @@ const _INACCESSIBLEMEM_BUILTINS = Any[
 ]
 
 const _ARGMEM_BUILTINS = Any[
-    memoryref,
+    memoryrefnew,
     memoryrefoffset,
     memoryrefget,
     memoryref_isassigned,
@@ -2503,7 +2503,7 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
     else
         if contains_is(_CONSISTENT_BUILTINS, f)
             consistent = ALWAYS_TRUE
-        elseif f === memoryref || f === memoryrefoffset
+        elseif f === memoryrefnew || f === memoryrefoffset
             consistent = ALWAYS_TRUE
         elseif f === memoryrefget || f === memoryrefset! || f === memoryref_isassigned
             consistent = CONSISTENT_IF_INACCESSIBLEMEMONLY
@@ -2527,7 +2527,7 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
         else
             inaccessiblememonly = ALWAYS_FALSE
         end
-        if f === memoryref || f === memoryrefget || f === memoryrefset! || f === memoryref_isassigned
+        if f === memoryrefnew || f === memoryrefget || f === memoryrefset! || f === memoryref_isassigned
             noub = memoryop_noub(f, argtypes) ? ALWAYS_TRUE : ALWAYS_FALSE
         else
             noub = ALWAYS_TRUE
@@ -2541,7 +2541,7 @@ function memoryop_noub(@nospecialize(f), argtypes::Vector{Any})
     nargs == 0 && return true # must throw and noub
     lastargtype = argtypes[end]
     isva = isvarargtype(lastargtype)
-    if f === memoryref
+    if f === memoryrefnew
         if nargs == 1 && !isva
             return true
         elseif nargs == 2 && !isva

diff --git a/base/deepcopy.jl b/base/deepcopy.jl
@@ -105,16 +105,16 @@ function _deepcopy_memory_t(@nospecialize(x::Memory), T, stackdict::IdDict)
     end
     dest = typeof(x)(undef, length(x))
     stackdict[x] = dest
-    xr = Core.memoryref(x)
-    dr = Core.memoryref(dest)
+    xr = memoryref(x)
+    dr = memoryref(dest)
     for i = 1:length(x)
-        xi = Core.memoryref(xr, i, false)
+        xi = Core.memoryrefnew(xr, i, false)
         if Core.memoryref_isassigned(xi, :not_atomic, false)
             xi = Core.memoryrefget(xi, :not_atomic, false)
             if !isbits(xi)
                 xi = deepcopy_internal(xi, stackdict)::typeof(xi)
             end
-            di = Core.memoryref(dr, i, false)
+            di = Core.memoryrefnew(dr, i, false)
             Core.memoryrefset!(di, xi, :not_atomic, false)
         end
     end
@@ -131,9 +131,9 @@ function deepcopy_internal(x::GenericMemoryRef, stackdict::IdDict)
         return stackdict[x]::typeof(x)
     end
     mem = getfield(x, :mem)
-    dest = GenericMemoryRef(deepcopy_internal(mem, stackdict)::typeof(mem))
+    dest = memoryref(deepcopy_internal(mem, stackdict)::typeof(mem))
     i = memoryrefoffset(x)
-    i == 1 || (dest = Core.memoryref(dest, i, true))
+    i == 1 || (dest = Core.memoryrefnew(dest, i, true))
     return dest
 end
 

diff --git a/base/docs/basedocs.jl b/base/docs/basedocs.jl
@@ -2683,23 +2683,23 @@ julia> Memory{Float64}(undef, 3)
 Memory{T}(::UndefInitializer, n)
 
 """
-    MemoryRef(memory)
+    `memoryref(::GenericMemory)`
 
-Construct a MemoryRef from a memory object. This does not fail, but the
-resulting memory may point out-of-bounds if the memory is empty.
+Construct a `GenericMemoryRef` from a memory object. This does not fail, but the
+resulting memory will point out-of-bounds if and only if the memory is empty.
 """
-MemoryRef(::Memory)
+memoryref(::Memory)
 
 """
-    MemoryRef(::Memory, index::Integer)
-    MemoryRef(::MemoryRef, index::Integer)
+    memorymef(::GenericMemory, index::Integer)
+    memoryref(::GenericMemoryRef, index::Integer)
 
-Construct a MemoryRef from a memory object and an offset index (1-based) which
+Construct a `GenericMemoryRef` from a memory object and an offset index (1-based) which
 can also be negative. This always returns an inbounds object, and will throw an
 error if that is not possible (because the index would result in a shift
 out-of-bounds of the underlying memory).
 """
-MemoryRef(::Union{Memory,MemoryRef}, ::Integer)
+memoryref(::Union{GenericMemory,GenericMemoryRef}, ::Integer)
 
 """
     Vector{T}(undef, n)

diff --git a/base/docs/intrinsicsdocs.jl b/base/docs/intrinsicsdocs.jl
@@ -23,20 +23,20 @@ The `Core.Intrinsics` module holds the `Core.IntrinsicFunction` objects.
 Core.Intrinsics
 
 """
-    Core.memoryref(::GenericMemory)
-    Core.memoryref(::GenericMemoryRef, index::Int, [boundscheck::Bool])
+    Core.memoryrefnew(::GenericMemory)
+    Core.memoryrefnew(::GenericMemoryRef, index::Int, [boundscheck::Bool])
 
-Return a `GenericMemoryRef` for a `GenericMemory`. See [`MemoryRef`](@ref).
+Return a `GenericMemoryRef` for a `GenericMemory`. See [`memoryref`](@ref).
 
 !!! compat "Julia 1.11"
     This function requires Julia 1.11 or later.
 """
-Core.memoryref
+Core.memoryrefnew
 
 """
     Core..memoryrefoffset(::GenericMemoryRef)
 
-Return the offset index that was used to construct the `MemoryRef`. See [`Core.memoryref`](@ref).
+Return the offset index that was used to construct the `MemoryRef`. See [`memoryref`](@ref).
 
 !!! compat "Julia 1.11"
     This function requires Julia 1.11 or later.

diff --git a/base/essentials.jl b/base/essentials.jl
@@ -1,6 +1,6 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
-using Core: CodeInfo, SimpleVector, donotdelete, compilerbarrier, memoryref, memoryrefget, memoryrefset!
+using Core: CodeInfo, SimpleVector, donotdelete, compilerbarrier, memoryrefnew, memoryrefget, memoryrefset!
 
 const Callable = Union{Function,Type}
 
@@ -361,7 +361,7 @@ default_access_order(a::GenericMemoryRef{:not_atomic}) = :not_atomic
 default_access_order(a::GenericMemoryRef{:atomic}) = :monotonic
 
 getindex(A::GenericMemory, i::Int) = (@_noub_if_noinbounds_meta;
-    memoryrefget(memoryref(memoryref(A), i, @_boundscheck), default_access_order(A), false))
+    memoryrefget(memoryrefnew(memoryrefnew(A), i, @_boundscheck), default_access_order(A), false))
 getindex(A::GenericMemoryRef) = memoryrefget(A, default_access_order(A), @_boundscheck)
 
 """
@@ -892,16 +892,16 @@ end
 function getindex(A::Array, i::Int)
     @_noub_if_noinbounds_meta
     @boundscheck ult_int(bitcast(UInt, sub_int(i, 1)), bitcast(UInt, length(A))) || throw_boundserror(A, (i,))
-    memoryrefget(memoryref(getfield(A, :ref), i, false), :not_atomic, false)
+    memoryrefget(memoryrefnew(getfield(A, :ref), i, false), :not_atomic, false)
 end
 # simple Array{Any} operations needed for bootstrap
 function setindex!(A::Array{Any}, @nospecialize(x), i::Int)
     @_noub_if_noinbounds_meta
     @boundscheck ult_int(bitcast(UInt, sub_int(i, 1)), bitcast(UInt, length(A))) || throw_boundserror(A, (i,))
-    memoryrefset!(memoryref(getfield(A, :ref), i, false), x, :not_atomic, false)
+    memoryrefset!(memoryrefnew(getfield(A, :ref), i, false), x, :not_atomic, false)
     return A
 end
-setindex!(A::Memory{Any}, @nospecialize(x), i::Int) = (memoryrefset!(memoryref(memoryref(A), i, @_boundscheck), x, :not_atomic, @_boundscheck); A)
+setindex!(A::Memory{Any}, @nospecialize(x), i::Int) = (memoryrefset!(memoryrefnew(memoryrefnew(A), i, @_boundscheck), x, :not_atomic, @_boundscheck); A)
 setindex!(A::MemoryRef{T}, x) where {T} = (memoryrefset!(A, convert(T, x), :not_atomic, @_boundscheck); A)
 setindex!(A::MemoryRef{Any}, @nospecialize(x)) = (memoryrefset!(A, x, :not_atomic, @_boundscheck); A)
 

diff --git a/base/exports.jl b/base/exports.jl
@@ -549,6 +549,7 @@ export
     mapfoldl,
     mapfoldr,
     mapreduce,
+    memoryref,
     merge!,
     mergewith!,
     merge,