Add Julia bindings design and in-repo Lattis.jl package with FFI wrapper and tests - #1
Add Julia bindings design and in-repo Lattis.jl package with FFI wrapper and tests#1wistaria wants to merge 1 commit into
Lattis.jl package with FFI wrapper and tests#1Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5f733c6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| function Basis(raw::Ptr{WrapFFI.lattis_basis_raw}) | ||
| obj = Basis(raw) |
There was a problem hiding this comment.
Avoid recursively calling the overwritten constructor
When basis_from_xml succeeds, it calls Basis(raw), but this outer constructor has the same signature as the default struct constructor and therefore replaces it; the obj = Basis(raw) call dispatches back to this same method until a stack overflow occurs instead of registering the finalizer. The identical pattern for Unitcell and Graph means the Julia bindings cannot construct any wrapped FFI object when LIBLATTIS_PATH is set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a design proposal and an in-repo Julia package skeleton (julia/Lattis.jl) that wraps the existing rust/lattis-ffi C ABI via ccall, providing Basis/Unitcell/Graph wrappers, XML round-trip helpers, and basic getters, plus an initial test suite and a README link to the design doc.
Changes:
- New design doc
docs/julia-bindings.mddescribing a two-layer Julia binding strategy over the existing C ABI. - New Julia package under
julia/Lattis.jlwithProject.toml, low-levelwrap_ffi.jl(ccall, library discovery, error retrieval, raw struct mirrors) and high-levelLattis.jl(mutable wrappers with finalizers,to_xml,close, getters). - Adds
julia/Lattis.jl/test/runtests.jlcovering FFI presence, XML round-trips, and basic getters; updatesREADME.mdwith a link to the design doc.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a pointer to the new Julia bindings design doc. |
| docs/julia-bindings.md | New design proposal for Julia bindings over rust/lattis-ffi. |
| julia/Lattis.jl/Project.toml | Package manifest for the new Julia package. |
| julia/Lattis.jl/src/wrap_ffi.jl | Low-level FFI layer: raw struct mirrors, library lookup, ccall wrappers, error retrieval. |
| julia/Lattis.jl/src/Lattis.jl | High-level wrappers, finalizers, to_xml/close/getters. |
| julia/Lattis.jl/test/runtests.jl | Unit tests for FFI presence, XML round-trip, and getters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sample = """ | ||
| <LATTICES> | ||
| <BASIS name=\"square\"><VECTOR>1 0</VECTOR><VECTOR>0 1</VECTOR></BASIS> | ||
| <UNITCELL name=\"uc\" dimension=\"2\"> | ||
| <VERTEX type=\"0\"><COORDINATE>0 0</COORDINATE></VERTEX> | ||
| </UNITCELL> | ||
| <LATTICEGRAPH name=\"g\" dimension=\"2\"> | ||
| <VERTEX type=\"0\"><COORDINATE>0 0</COORDINATE></VERTEX> | ||
| </LATTICEGRAPH> | ||
| </LATTICES> | ||
| """ | ||
|
|
||
| b = basis_from_xml(sample, "square") | ||
| @test dimension(b) == 2 | ||
| @test occursin("BASIS", to_xml("square2", b)) | ||
| close(b) | ||
|
|
||
| u = unitcell_from_xml(sample, "uc") | ||
| @test dimension(u) == 2 | ||
| @test num_sites(u) == 1 | ||
| @test num_bonds(u) == 0 | ||
| @test occursin("UNITCELL", to_xml("uc2", u)) | ||
| close(u) | ||
|
|
||
| g = graph_from_xml(sample, "g") | ||
| @test dimension(g) == 2 | ||
| @test num_sites(g) == 1 | ||
| @test num_bonds(g) == 0 | ||
| @test occursin("LATTICEGRAPH", to_xml("g2", g)) |
| end | ||
|
|
||
| mutable struct Unitcell | ||
| raw::Ptr{WrapFFI.lattis_unitcell_raw} | ||
| end | ||
|
|
||
| mutable struct Graph | ||
| raw::Ptr{WrapFFI.lattis_graph_raw} | ||
| end | ||
|
|
||
|
|
||
| function Basis(raw::Ptr{WrapFFI.lattis_basis_raw}) | ||
| obj = Basis(raw) | ||
| finalizer(obj) do x | ||
| close(x) | ||
| end | ||
| return obj | ||
| end | ||
|
|
||
| function Unitcell(raw::Ptr{WrapFFI.lattis_unitcell_raw}) | ||
| obj = Unitcell(raw) | ||
| finalizer(obj) do x | ||
| close(x) | ||
| end | ||
| return obj | ||
| end | ||
|
|
||
| function Graph(raw::Ptr{WrapFFI.lattis_graph_raw}) | ||
| obj = Graph(raw) | ||
| finalizer(obj) do x | ||
| close(x) | ||
| end | ||
| return obj | ||
| end | ||
|
|
| function close(b::Basis) | ||
| if b.raw != C_NULL | ||
| WrapFFI.free_basis_raw(b.raw) | ||
| b.raw = Ptr{WrapFFI.lattis_basis_raw}(C_NULL) | ||
| end | ||
| return nothing | ||
| end | ||
|
|
||
| function close(u::Unitcell) | ||
| if u.raw != C_NULL | ||
| WrapFFI.free_unitcell_raw(u.raw) | ||
| u.raw = Ptr{WrapFFI.lattis_unitcell_raw}(C_NULL) | ||
| end | ||
| return nothing | ||
| end | ||
|
|
||
| function close(g::Graph) | ||
| if g.raw != C_NULL | ||
| WrapFFI.free_graph_raw(g.raw) | ||
| g.raw = Ptr{WrapFFI.lattis_graph_raw}(C_NULL) | ||
| end | ||
| return nothing | ||
| end |
| name = "Lattis" | ||
| uuid = "7d2e9a9b-7e38-4b40-8e68-2ce6ff8d5b66" | ||
| authors = ["Lattis contributors"] | ||
| version = "0.1.0" | ||
|
|
||
| [compat] | ||
| julia = ">=1.9" | ||
|
|
||
| [extras] | ||
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | ||
|
|
||
| [targets] | ||
| test = ["Test"] |
| FFI returns status codes (`0` success, non-zero failure). On failure: | ||
|
|
||
| 1. Query thread-local/last error message from FFI. | ||
| 2. Convert to Julia exception (`ErrorException` or custom `LattisError`). | ||
|
|
||
| Helper pattern: | ||
|
|
||
| * `check(code::Cint)` throws on non-zero | ||
| * all high-level constructors/methods call `check` |
Motivation
lattiscore while reusing the existing C ABI (rust/lattis-ffi) to avoid duplicating logic and to keep Rust internals private.Description
docs/julia-bindings.mddescribing the recommended two-layer binding strategy, ABI choices, memory/error conventions, and roadmap.julia/Lattis.jlincludingProject.toml,src/Lattis.jl(high-level wrappers and finalizers), andsrc/wrap_ffi.jl(low-levelccalllayer withliblattis_ffidiscovery and error handling).basis_from_xml,unitcell_from_xml,graph_from_xml),to_xml,close, and simple getters (dimension,num_sites,num_bonds) plus library lookup helpers (liblattis_path,has_liblattis).julia/Lattis.jl/test/runtests.jlexercising FFI presence checks, XML round-trips, and basic property getters, and updateREADME.mdwith a link to the Julia binding proposal.Testing
julia/Lattis.jl/test/runtests.jlthat validate FFI availability, XML parsing/serialization, and basic getters but these tests were not executed as part of this PR.julia --project=julia/Lattis.jl -e 'using Pkg; Pkg.test()'withLIBLATTIS_PATHpointing to the builtliblattis_ffishared library.Codex Task