avoid defining convert(Vector{String}, ...) in LibGit2 (#56082)

This is a weird conversion function to define. Seems cleaner to use the iteration interface for this. Also avoids some invalidations (#56080 (comment)) Co-authored-by: KristofferC <[email protected]>
JuliaLang · Oct 10, 2024 · a007e80 · a007e80
1 parent dc609a7
commit a007e80
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 10 deletions.
diff --git a/stdlib/LibGit2/src/reference.jl b/stdlib/LibGit2/src/reference.jl
@@ -215,7 +215,7 @@ function ref_list(repo::GitRepo)
     sa_ref = Ref(StrArrayStruct())
     @check ccall((:git_reference_list, libgit2), Cint,
                       (Ptr{StrArrayStruct}, Ptr{Cvoid}), sa_ref, repo)
-    res = convert(Vector{String}, sa_ref[])
+    res = collect(sa_ref[])
     free(sa_ref)
     res
 end

diff --git a/stdlib/LibGit2/src/remote.jl b/stdlib/LibGit2/src/remote.jl
@@ -215,7 +215,7 @@ function fetch_refspecs(rmt::GitRemote)
     sa_ref = Ref(StrArrayStruct())
     @check ccall((:git_remote_get_fetch_refspecs, libgit2), Cint,
                  (Ptr{StrArrayStruct}, Ptr{Cvoid}), sa_ref, rmt)
-    res = convert(Vector{String}, sa_ref[])
+    res = collect(sa_ref[])
     free(sa_ref)
     res
 end
@@ -245,7 +245,7 @@ function push_refspecs(rmt::GitRemote)
     sa_ref = Ref(StrArrayStruct())
     @check ccall((:git_remote_get_push_refspecs, libgit2), Cint,
                  (Ptr{StrArrayStruct}, Ptr{Cvoid}), sa_ref, rmt)
-    res = convert(Vector{String}, sa_ref[])
+    res = collect(sa_ref[])
     free(sa_ref)
     res
 end

diff --git a/stdlib/LibGit2/src/repository.jl b/stdlib/LibGit2/src/repository.jl
@@ -518,7 +518,7 @@ function remotes(repo::GitRepo)
     @assert repo.ptr != C_NULL
     @check ccall((:git_remote_list, libgit2), Cint,
                   (Ptr{StrArrayStruct}, Ptr{Cvoid}), sa_ref, repo)
-    res = convert(Vector{String}, sa_ref[])
+    res = collect(sa_ref[])
     free(sa_ref)
     return res
 end

diff --git a/stdlib/LibGit2/src/strarray.jl b/stdlib/LibGit2/src/strarray.jl
@@ -1,6 +1,5 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
-
 function Base.cconvert(::Type{Ptr{StrArrayStruct}}, x::Vector)
     str_ref = Base.cconvert(Ref{Cstring}, x)
     sa_ref = Ref(StrArrayStruct(Base.unsafe_convert(Ref{Cstring}, str_ref), length(x)))
@@ -10,6 +9,8 @@ function Base.unsafe_convert(::Type{Ptr{StrArrayStruct}}, rr::Tuple{Ref{StrArray
     Base.unsafe_convert(Ptr{StrArrayStruct}, first(rr))
 end
 
-function Base.convert(::Type{Vector{String}}, sa::StrArrayStruct)
-    [unsafe_string(unsafe_load(sa.strings, i)) for i = 1:sa.count]
+Base.length(sa::StrArrayStruct) = sa.count
+function Base.iterate(sa::StrArrayStruct, state=1)
+    state > sa.count && return nothing
+    (unsafe_string(unsafe_load(sa.strings, state)), state+1)
 end
diff --git a/stdlib/LibGit2/src/tag.jl b/stdlib/LibGit2/src/tag.jl
@@ -10,7 +10,7 @@ function tag_list(repo::GitRepo)
     sa_ref = Ref(StrArrayStruct())
     @check ccall((:git_tag_list, libgit2), Cint,
                  (Ptr{StrArrayStruct}, Ptr{Cvoid}), sa_ref, repo)
-    res = convert(Vector{String}, sa_ref[])
+    res = collect(sa_ref[])
     free(sa_ref)
     res
 end

diff --git a/stdlib/LibGit2/src/types.jl b/stdlib/LibGit2/src/types.jl
@@ -78,7 +78,7 @@ When fetching data from LibGit2, a typical usage would look like:
 ```julia
 sa_ref = Ref(StrArrayStruct())
 @check ccall(..., (Ptr{StrArrayStruct},), sa_ref)
-res = convert(Vector{String}, sa_ref[])
+res = collect(sa_ref[])
 free(sa_ref)
 ```
 In particular, note that `LibGit2.free` should be called afterward on the `Ref` object.

diff --git a/stdlib/LibGit2/test/libgit2-tests.jl b/stdlib/LibGit2/test/libgit2-tests.jl
@@ -95,7 +95,7 @@ end
     p = ["XXX","YYY"]
     a = Base.cconvert(Ptr{LibGit2.StrArrayStruct}, p)
     b = Base.unsafe_convert(Ptr{LibGit2.StrArrayStruct}, a)
-    @test p == convert(Vector{String}, unsafe_load(b))
+    @test p == collect(unsafe_load(b))
     @noinline gcuse(a) = a
     gcuse(a)
 end