use CheckedSizeProduct.jl for implementing checked_dims

[nsajko]

Better effects are a benefit.

[nsajko]

The package is not registered yet, so this will fail CI, marking as draft. Submitted the PR to get possible input on CheckedSizeProduct before registration.

[nsajko]

I decided not to change the error message contents right away to keep the diff small. That said, CheckedSizeProduct supports distinguishing between "invalid value" (negative or typemax) and "overflow" conditions.

[LilithHafner]

I'd be willing to give feedback on CheckedSizeProduct.jl. I appreciate your effort making a package for this.

I also request that you take more time to verify claims you make when opening PRs to this package. Though I do realize it is harder to test this PR given that CheckedSizeProduct isn't registered.

It would be easy to add more precise error messages now.

AFAICT It would be equally easy as it is right now. The flags o, neg, and overflow & !zero in main correspond to the flag o, type CheckedSizeProduct.StatusInvalidValue, and type CheckedSizeProduct.StatusOverflow in the pr.

Consistency with Array: typemax(T) is not allowed as a size any more.

typemax(T) is already not allowed.

julia> malloc(UInt8, typemax(Int))
ERROR: ArgumentError: invalid malloc dimensions
Stacktrace:
 [1] #checked_dims#4
   @ ~/.julia/packages/PtrArrays/UrKqT/src/PtrArrays.jl:47 [inlined]
 [2] checked_dims
   @ ~/.julia/packages/PtrArrays/UrKqT/src/PtrArrays.jl:34 [inlined]
 [3] malloc(::Type{UInt8}, dims::Int64)
   @ PtrArrays ~/.julia/packages/PtrArrays/UrKqT/src/PtrArrays.jl:63
 [4] top-level scope
   @ REPL[29]:1

Better effects.

This PR does not change the effects of PtrArrays.checked_product or malloc.

julia> PtrArrays.CheckedSizeProduct
ERROR: UndefVarError: `CheckedSizeProduct` not defined in `PtrArrays`
Suggestion: check for spelling errors or missing imports.
Hint: a global variable of this name also exists in CheckedSizeProduct.
Stacktrace:
 [1] getproperty(x::Module, f::Symbol)
   @ Base ./Base.jl:42
 [2] top-level scope
   @ REPL[30]:1

julia> Base.infer_effects(PtrArrays.checked_dims, Tuple{Int, Int})
(+c,+e,!n,+t,+s,+m,+u,+o,+r)

julia> Base.infer_effects(malloc, Tuple{Type{Int}, Int, Int})
(!c,!e,!n,!t,!s,!m,!u,!o,!r)

julia> PtrArrays.CheckedSizeProduct
CheckedSizeProduct

julia> Base.infer_effects(PtrArrays.checked_dims, Tuple{Int, Int})
(+c,+e,!n,+t,+s,+m,+u,+o,+r)

julia> Base.infer_effects(malloc, Tuple{Type{Int}, Int, Int})
(!c,!e,!n,!t,!s,!m,!u,!o,!r)

[nsajko]

AFAICT It would be equally easy as it is right now.

You're absolutely right about the flags having the same meaning, and I failed to interpret your neg correctly. Sorry. I think it might make sense to change CheckedSizeProduct to use x -> (x + 1) < 1 instead of x -> x < 0 and x -> x == typemax(x), like here.

typemax(T) is already not allowed.

I'm sorry, I misinterpreted your implementation and failed to check my assumptions in the REPL.

This PR does not change the effects

You forgot about the mandatory keyword argument to checked_dims when calling infer_effects. The result is that Julia infers "throws unconditionally", so :total except for :nothrow, but that's not the call you're interested in.

On Julia v1.11.2 and PtrArrays main branch:

julia> InteractiveUtils.@infer_effects PtrArrays.checked_dims(0, 0, 0, 0; message = :malloc)
(+c,!e,!n,!t,!s,!m,!u,!o,!r)

On Julia v1.11.2 and PR branch:

julia> InteractiveUtils.@infer_effects PtrArrays.checked_dims(0, 0, 0, 0; message = :malloc)
(+c,+e,!n,+t,!s,!m,+u,+o,+r)

The result is that, e.g., checked_dims is constant folded from a malloc call when all the dimension sizes are known, check with:

using Cthulhu, PtrArrays
function func()
    malloc(Int, 2, 2, 2)
end
descend(func, Tuple{})  # use the `code_typed` mode, accessible by pressing `T`

I know this is a small benefit, but I wanted to get it right given that the package is so small anyway.

[LilithHafner]

You're right about the effects, thanks! Another way to see the impact on effects is

julia> @b free(malloc(Int, 2, 2, 2))
577.381 ns (8 allocs: 224 bytes) # main
7.937 ns # pr

FWIW, the impact is only for >=3 dimensions and is due to JuliaLang/julia#57097

[giordano]

The package is not registered yet, so this will fail CI, marking as draft. Submitted the PR to get possible input on CheckedSizeProduct before registration.

You can add a step before

PtrArrays.jl/.github/workflows/CI.yml

Line 47 in ef99c9f

- uses: julia-actions/julia-runtest@v1

which does

import Pkg
Pkg.add(; url="https://github.com/JuliaArrays/CheckedSizeProduct.jl")

and then you can actually run CI until the package is registered.