tweak pyconvert

Author: Christopher Doris

docs/src/releasenotes.md

@@ -1,8 +1,9 @@
 # Release Notes
 
 ## Unreleased
-* `pyconvert_add_rule` is now documented. The separator of the first argument has changed
-  from `/` to `:` (not breaking because it was previously undocumented).
+* `pyconvert_add_rule` is now documented. Its semantics have changed, including the
+  separator of the first argument from `/` to `:` (not breaking because it was previously
+  undocumented).
 
 ## v0.6.1 (2022-02-21)
 * Conversions from simple ctypes types, e.g. `ctypes.c_float` to `Cfloat`.

src/concrete/bool.jl

@@ -23,18 +23,10 @@ pybool_asbool(x) =
 
 function pyconvert_rule_bool(::Type{T}, x::Py) where {T<:Number}
     val = pybool_asbool(x)
+    pydel!(x)
     if T in (Bool, Int8, Int16, Int32, Int64, Int128, UInt8, UInt16, UInt32, UInt64, UInt128, BigInt)
         pyconvert_return(T(val))
     else
         pyconvert_tryconvert(T, val)
     end
 end
-
-pyconvert_rule_fast(::Type{Bool}, x::Py) =
-    if pyisTrue(x)
-        pyconvert_return(true)
-    elseif pyisFalse(x)
-        pyconvert_return(false)
-    else
-        pyconvert_unconverted()
-    end

src/concrete/bytes.jl

@@ -35,6 +35,3 @@ end
 
 pyconvert_rule_bytes(::Type{Vector{UInt8}}, x::Py) = pyconvert_return(copy(pybytes_asvector(x)))
 pyconvert_rule_bytes(::Type{Base.CodeUnits{UInt8,String}}, x::Py) = pyconvert_return(codeunits(pybytes_asUTF8string(x)))
-
-pyconvert_rule_fast(::Type{Vector{UInt8}}, x::Py) = pyisbytes(x) ? pyconvert_rule_bytes(Vector{UInt8}, x) : pyconvert_unconverted()
-pyconvert_rule_fast(::Type{Base.CodeUnits{UInt8,String}}, x::Py) = pyisbytes(x) ? pyconvert_rule_bytyes(Base.CodeUnits{UInt8,String}, x) : pyconvert_unconverted()

src/concrete/complex.jl

@@ -31,10 +31,3 @@ function pyconvert_rule_complex(::Type{T}, x::Py) where {T<:Number}
         pyconvert_tryconvert(T, val)
     end
 end
-
-pyconvert_rule_fast(::Type{Complex{Float64}}, x::Py) =
-    if pyiscomplex(x)
-        pyconvert_return(pycomplex_ascomplex(x))
-    else
-        pyconvert_unconverted()
-    end

src/concrete/float.jl

@@ -16,6 +16,7 @@ pyfloat_asdouble(x) = errcheck_ambig(@autopy x C.PyFloat_AsDouble(getptr(x_)))
 
 function pyconvert_rule_float(::Type{T}, x::Py) where {T<:Number}
     val = pyfloat_asdouble(x)
+    pydel!(x)
     if T in (Float16, Float32, Float64, BigFloat)
         pyconvert_return(T(val))
     else
@@ -26,24 +27,21 @@ end
 # NaN is sometimes used to represent missing data of other types
 # so we allow converting it to Nothing or Missing
 function pyconvert_rule_float(::Type{Nothing}, x::Py)
-    if isnan(pyfloat_asdouble(x))
+    val = pyfloat_asdouble(x)
+    pydel!(x)
+    if isnan(val)
         pyconvert_return(nothing)
     else
         pyconvert_unconverted()
     end
 end
 
 function pyconvert_rule_float(::Type{Missing}, x::Py)
-    if isnan(pyfloat_asdouble(x))
+    val = pyfloat_asdouble(x)
+    pydel!(x)
+    if isnan(val)
         pyconvert_return(missing)
     else
         pyconvert_unconverted()
     end
 end
-
-pyconvert_rule_fast(::Type{Float64}, x::Py) =
-    if pyisfloat(x)
-        pyconvert_return(pyfloat_asdouble(x))
-    else
-        pyconvert_unconverted()
-    end

src/concrete/int.jl

@@ -39,6 +39,7 @@ pyconvert_rule_int(::Type{T}, x::Py) where {T<:Number} = begin
     v = T <: Unsigned ? C.PyLong_AsUnsignedLongLong(getptr(x)) : C.PyLong_AsLongLong(getptr(x))
     if !iserrset_ambig(v)
         # success
+        pydel!(x)
         return pyconvert_tryconvert(T, v)
     elseif errmatches(pybuiltins.OverflowError)
         # overflows Clonglong or Culonglong
@@ -59,17 +60,20 @@ pyconvert_rule_int(::Type{T}, x::Py) where {T<:Number} = begin
            typemin(typeof(v)) ≤ typemin(T) &&
            typemax(T) ≤ typemax(typeof(v))
             # definitely overflows S, give up now
+            pydel!(x)
             return pyconvert_unconverted()
         else
             # try converting -> int -> str -> BigInt -> T
             x_int = pyint(x)
+            pydel!(x)
             x_str = pystr(String, x_int)
             pydel!(x_int)
             v = parse(BigInt, x_str)
             return pyconvert_tryconvert(T, v)
         end
     else
         # other error
+        pydel!(x)
         pythrow()
     end
 end

src/concrete/none.jl

@@ -2,17 +2,3 @@ pyisnone(x) = pyis(x, pybuiltins.None)
 
 pyconvert_rule_none(::Type{Nothing}, x::Py) = pyconvert_return(nothing)
 pyconvert_rule_none(::Type{Missing}, x::Py) = pyconvert_return(missing)
-
-pyconvert_rule_fast(::Type{Nothing}, x::Py) =
-    if pyisnone(x)
-        pyconvert_return(nothing)
-    else
-        pyconvert_unconverted()
-    end
-
-pyconvert_rule_fast(::Type{Missing}, x::Py) =
-    if pyisnone(x)
-        pyconvert_return(missing)
-    else
-        pyconvert_unconverted()
-    end

src/concrete/range.jl

@@ -31,6 +31,3 @@ function pyconvert_rule_range(::Type{R}, x::Py, ::Type{UnitRange{T0}}=Utils._typ
     T2 = Utils._promote_type_bounded(T0, typeof(a′), typeof(c′), T1)
     pyconvert_return(UnitRange{T2}(a′, c′))
 end
-
-pyconvert_rule_fast(::Type{T}, x::Py) where {T<:StepRange} = pyisrange(x) ? pyconvert_rule_range(T, x) : pyconvert_unconverted()
-pyconvert_rule_fast(::Type{T}, x::Py) where {T<:UnitRange} = pyisrange(x) ? pyconvert_rule_range(T, x) : pyconvert_unconverted()

src/concrete/str.jl

@@ -35,7 +35,3 @@ pyconvert_rule_str(::Type{Char}, x::Py) = begin
 end
 
 pyisstr(x) = pytypecheckfast(x, C.Py_TPFLAGS_UNICODE_SUBCLASS)
-
-pyconvert_rule_fast(::Type{String}, x::Py) = pyisstr(x) ? pyconvert_rule_str(String, x) : pyconvert_unconverted()
-pyconvert_rule_fast(::Type{Symbol}, x::Py) = pyisstr(x) ? pyconvert_rule_str(Symbol, x) : pyconvert_unconverted()
-pyconvert_rule_fast(::Type{Char}, x::Py) = pyisstr(x) ? pyconvert_rule_str(Char, x) : pyconvert_unconverted()

src/convert.jl

@@ -63,39 +63,41 @@ Other priorities are reserved for internal use.
 
 ### Optional optimization (for experts)
 
-If `func(S, x::Py)` makes a successful conversion, it may call `pydel!(x)` if safe to do so.
-This means that the returned answer must not reference `x` in any way.
+The `func` is given the only reference to `x`. This means it may call `pydel!(x)` when done
+with `x` provided `x` is not referenced anywhere else, such as in the return value. See
+[`pydel!`](@ref).
 
 If you are returning a wrapper type, such as `PyList(x)`, then the object `x` should be
 duplicated first, as in `Py(x)`. It is recommended to force this in the inner constructor
 of the wrapper type.
-
-If the conversion is not successful, it **must not** call `pydel!(x)`.
 """
 function pyconvert_add_rule(pytypename::String, type::Type, func::Function, priority::PyConvertPriority=PYCONVERT_PRIORITY_NORMAL)
     @nospecialize type func
     push!(get!(Vector{PyConvertRule}, PYCONVERT_RULES, pytypename), PyConvertRule(type, func, priority))
     return
 end
 
-if false
-    # this scheme returns either the result or Unconverted()
+# Alternative ways to represent the result of conversion.
+if true
+    # Returns either the result or Unconverted().
     struct Unconverted end
-    pyconvert_return(x) = x
-    pyconvert_unconverted() = Unconverted()
-    pyconvert_returntype(::Type{T}) where {T} = Union{T,Unconverted}
-    pyconvert_isunconverted(r) = r === Unconverted()
-    pyconvert_result(::Type{T}, r) where {T} = r::T
-elseif true
-    # this scheme stores the result in PYCONVERT_RESULT
+    @inline pyconvert_return(x) = x
+    @inline pyconvert_unconverted() = Unconverted()
+    @inline pyconvert_returntype(::Type{T}) where {T} = Union{T,Unconverted}
+    @inline pyconvert_isunconverted(r) = r === Unconverted()
+    @inline pyconvert_result(::Type{T}, r) where {T} = r::T
+elseif false
+    # Stores the result in PYCONVERT_RESULT.
+    # This is global state, probably best avoided.
     const PYCONVERT_RESULT = Ref{Any}(nothing)
-    pyconvert_return(x) = (PYCONVERT_RESULT[] = x; true)
-    pyconvert_unconverted() = false
-    pyconvert_returntype(::Type{T}) where {T} = Bool
-    pyconvert_isunconverted(r::Bool) = !r
-    pyconvert_result(::Type{T}, r::Bool) where {T} = (ans = PYCONVERT_RESULT[]::T; PYCONVERT_RESULT[] = nothing; ans)
+    @inline pyconvert_return(x) = (PYCONVERT_RESULT[] = x; true)
+    @inline pyconvert_unconverted() = false
+    @inline pyconvert_returntype(::Type{T}) where {T} = Bool
+    @inline pyconvert_isunconverted(r::Bool) = !r
+    @inline pyconvert_result(::Type{T}, r::Bool) where {T} = (ans = PYCONVERT_RESULT[]::T; PYCONVERT_RESULT[] = nothing; ans)
 else
-    # same as the previous scheme, but with special handling for bits types
+    # Same as the previous scheme, but with special handling for bits types.
+    # This is global state, probably best avoided.
     const PYCONVERT_RESULT = Ref{Any}(nothing)
     const PYCONVERT_RESULT_ISBITS = Ref{Bool}(false)
     const PYCONVERT_RESULT_TYPE = Ref{Type}(Union{})
@@ -112,9 +114,9 @@ else
         end
         return true
     end
-    pyconvert_unconverted() = false
-    pyconvert_returntype(::Type{T}) where {T} = Bool
-    pyconvert_isunconverted(r::Bool) = !r
+    @inline pyconvert_unconverted() = false
+    @inline pyconvert_returntype(::Type{T}) where {T} = Bool
+    @inline pyconvert_isunconverted(r::Bool) = !r
     function pyconvert_result(::Type{T}, r::Bool) where {T}
         if isbitstype(T)
             if sizeof(T) ≤ PYCONVERT_RESULT_BITSLEN
@@ -271,40 +273,63 @@ end
 
 pyconvert_fix(::Type{T}, func) where {T} = x -> func(T, x)
 
-const PYCONVERT_RULES_CACHE = Dict{C.PyPtr, Dict{Type, Vector{Function}}}()
+const PYCONVERT_RULES_CACHE = Dict{Type, Dict{C.PyPtr, Vector{Function}}}()
+
+@generated pyconvert_rules_cache(::Type{T}) where {T} = get!(Dict{C.PyPtr, Vector{Function}}, PYCONVERT_RULES_CACHE, T)
 
 function pyconvert_rule_fast(::Type{T}, x::Py) where {T}
     if T isa Union
         a = pyconvert_rule_fast(T.a, x) :: pyconvert_returntype(T.a)
         pyconvert_isunconverted(a) || return a
         b = pyconvert_rule_fast(T.b, x) :: pyconvert_returntype(T.b)
         pyconvert_isunconverted(b) || return b
+    elseif (T == Nothing) | (T == Missing)
+        pyisnone(x) && return pyconvert_return(T())
+    elseif (T == Bool)
+        pyisFalse(x) && return pyconvert_return(false)
+        pyisTrue(x) && return pyconvert_return(true)
+    elseif (T == Int) | (T == BigInt)
+        pyisint(x) && return pyconvert_rule_int(T, Py(x))
+    elseif (T == Float64)
+        pyisfloat(x) && return pyconvert_return(T(pyfloat_asdouble(x)))
+    elseif (T == ComplexF64)
+        pyiscomplex(x) && return pyconvert_return(T(pycomplex_ascomplex(x)))
+    elseif (T == String) | (T == Char) | (T == Symbol)
+        pyisstr(x) && return pyconvert_rule_str(T, Py(x))
+    elseif (T == Vector{UInt8}) | (T == Base.CodeUnits{UInt8,String})
+        pyisbytes(x) && return pyconvert_rule_bytes(T, Py(x))
+    elseif (T <: StepRange) | (T <: UnitRange)
+        pyisrange(x) && return pyconvert_rule_range(T, Py(x))
     end
     pyconvert_unconverted()
 end
 
 function pytryconvert(::Type{T}, x_) where {T}
     # Copy the input
     x = Py(x_)
+
     # We can optimize the conversion for some types by overloading pytryconvert_fast.
     # It MUST give the same results as via the slower route using rules.
-    ans = pyconvert_rule_fast(T, x) :: pyconvert_returntype(T)
-    pyconvert_isunconverted(ans) || return ans
+    ans1 = pyconvert_rule_fast(T, x) :: pyconvert_returntype(T)
+    pyconvert_isunconverted(ans1) || (pydel!(x); return ans1)
+
     # get rules from the cache
     # TODO: we should hold weak references and clear the cache if types get deleted
     tptr = C.Py_Type(getptr(x))
-    trules = get!(Dict{Type, Vector{PyConvertRule}}, PYCONVERT_RULES_CACHE, tptr)
-    if !haskey(trules, T)
+    trules = pyconvert_rules_cache(T)
+    rules = get!(trules, tptr) do
         t = pynew(incref(tptr))
-        trules[T] = pyconvert_get_rules(T, t)
+        ans = pyconvert_get_rules(T, t)::Vector{Function}
         pydel!(t)
+        ans
     end
-    rules = trules[T]
+
     # apply the rules
     for rule in rules
-        ans = rule(x) :: pyconvert_returntype(T)
-        pyconvert_isunconverted(ans) || return ans
+        ans2 = rule(Py(x)) :: pyconvert_returntype(T)
+        pyconvert_isunconverted(ans2) || (pydel!(x); return ans2)
     end
+
     # if no rule succeeded, assume nothing references x
     pydel!(x)
     return pyconvert_unconverted()