Skip to content

Commit

Permalink
Fix more JET issues (#951)
Browse files Browse the repository at this point in the history
Some found via @report_opt
  • Loading branch information
fingolfin authored Nov 17, 2023
1 parent 48d13e4 commit 29d6b87
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/GAP.jl
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ function error_handler()
if disable_error_handler[]
return
end
last_error[] = String(Globals._JULIAINTERFACE_ERROR_BUFFER)
ccall((:SET_LEN_STRING, libgap), Cvoid, (GapObj, Cuint), Globals._JULIAINTERFACE_ERROR_BUFFER, 0)
last_error[] = String(Globals._JULIAINTERFACE_ERROR_BUFFER::GapObj)
ccall((:SET_LEN_STRING, libgap), Cvoid, (GapObj, Cuint), Globals._JULIAINTERFACE_ERROR_BUFFER::GapObj, 0)
end

function ThrowObserver(depth::Cint)
Expand Down
2 changes: 1 addition & 1 deletion src/adapter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function show_string(io::IO, obj::Union{GapObj,FFE})
str = Wrappers.StringViewObj(obj)
stri = CSTR_STRING(str)
lines = split(stri, "\n")
rows = displaysize(io)[1]-3 # the maximum number of lines to show
rows = displaysize(io)[1]::Int - 3 # the maximum number of lines to show
if length(lines) > rows
# For objects that do not fit on the screen,
# show only the first and the last lines.
Expand Down
6 changes: 3 additions & 3 deletions src/ccalls.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,17 @@ whereas `x` in the latter example lives in the Julia session.
"""
function evalstr(cmd::String)
res = evalstr_ex(cmd * ";")
if any(x->x[1] == false, res)
if any(x::GapObj->x[1] === false, res)
# error
global last_error
# HACK HACK HACK: if there is an error string on the GAP side, call
# error_handler to copy it into `last_error`
if !Wrappers.IsEmpty(Globals._JULIAINTERFACE_ERROR_BUFFER)
if !Wrappers.IsEmpty(Globals._JULIAINTERFACE_ERROR_BUFFER::GapObj)
error_handler()
end
error("Error thrown by GAP: $(last_error[])")
end
res = res[end]
res = res[end]::GapObj
if Wrappers.ISB_LIST(res, 2)
return res[2]
else
Expand Down
4 changes: 2 additions & 2 deletions src/constructors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ julia> String(val) # an empty GAP list is a string
"""
function String(obj::GapObj)
Wrappers.IsStringRep(obj) && return CSTR_STRING(obj)
Wrappers.IsString(obj) && return CSTR_STRING(Wrappers.CopyToStringRep(obj))
Wrappers.IsString(obj) && return CSTR_STRING(Wrappers.CopyToStringRep(obj)::GapObj)
throw(ConversionError(obj, String))
end

Expand Down Expand Up @@ -257,7 +257,7 @@ function BitVector(obj::GapObj)
len = Wrappers.Length(obj)::Int
result = BitVector(undef, len)
for i = 1:len
result[i] = obj[i]
result[i] = obj[i]::Bool
end
return result
end
Expand Down
7 changes: 6 additions & 1 deletion src/gap_to_julia.jl
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,12 @@ end
## for the GAP function GAPToJulia:
## turning arguments into keyword arguments is easier in Julia than in GAP
_gap_to_julia(x::Obj) = gap_to_julia(x)
_gap_to_julia(x::Obj, recursive::Bool) = gap_to_julia(x; recursive)

_gap_to_julia(x::Bool, recursive::Bool) = x
_gap_to_julia(x::Int, recursive::Bool) = x
_gap_to_julia(x::FFE, recursive::Bool) = x
_gap_to_julia(x::GapObj, recursive::Bool) = gap_to_julia(x; recursive)

_gap_to_julia(::Type{T}, x::Obj) where {T} = gap_to_julia(T, x)
_gap_to_julia(::Type{T}, x::Obj, recursive::Bool) where {T} =
gap_to_julia(T, x; recursive)
3 changes: 2 additions & 1 deletion src/julia_to_gap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ function julia_to_gap(
elseif Wrappers.IsRecord(obj)
ret_val = NewPrecord(0)
recursion_dict[obj] = ret_val
for x in Vector{String}(Wrappers.RecNames(obj))
for xx in Wrappers.RecNames(obj)::GapObj
x = Wrappers.RNamObj(xx)
Wrappers.ASS_REC(ret_val, x, julia_to_gap(Wrappers.ELM_REC(obj, x), recursion_dict; recursive = true))
end
else
Expand Down
11 changes: 8 additions & 3 deletions test/conversion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,17 @@ end

@testset "Dictionaries" begin
x = GAP.evalstr("rec( foo := 1, bar := \"foo\" )")
# ... recursive conversion
y = Dict{Symbol,Any}(:foo => 1, :bar => "foo")
z = GAP.evalstr("rec( foo := 1, bar := JuliaEvalString(\"\\\"foo\\\"\") )")
# ... recursive conversion
@test GAP.julia_to_gap(y; recursive = true) == x
# ... non-recursive conversion
x = GAP.evalstr("rec( foo := 1, bar := JuliaEvalString(\"\\\"foo\\\"\") )")
@test GAP.julia_to_gap(y) == x
@test GAP.julia_to_gap(y) == z

# also test the case were the top level is a GapObj but inside
# there are Julia objects
@test GAP.julia_to_gap(z; recursive=true) == x
@test GAP.julia_to_gap(z; recursive=false) == z # nothing happens without recursion
end

@testset "Conversions with identical sub-objects" begin
Expand Down

0 comments on commit 29d6b87

Please sign in to comment.