GValue runtime wrappers + enum decoders + signal_marshaller#129
Merged
chris-armstrong merged 10 commits intoMay 3, 2026
Conversation
Stage 1 of milestone-2-signals Phase 1a. Adds C primitive ml_g_value_set_object_null and dispatches the None branch of Value.set_object to it, replacing the previous no-op that left the GValue uninitialised. Adds a regression test in test_gobj.ml using Alcotest.(option pass) and unblocks the previously commented-out test_gobj stanza in ocgtk/tests/dune. Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a) Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 1) Chris Armstrong
…, flags, boxed Stage 2 of milestone-2-signals Phase 1a. Adds 10 new generic primitives in ml_gobject.c covering int64, variant, enum_int, flags_int, and boxed-record GValue payloads. The boxed pair reuses existing ocgtk_gir_record_ops via ml_gir_record_val_ptr_with_type / ml_gir_record_ptr_val (no new ops record); g_boxed_copy in get_boxed makes the wrapper take ownership for safe finalization through the existing GType-aware finalizer. The OCaml externals are added in Stage 3. Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a) Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 2) Chris Armstrong
Stage 3 of milestone-2-signals Phase 1a. Adds 10 OCaml externals in
gobject.{ml,mli} module Value matching the C primitives committed in
35712f5: int64, variant, enum_int, flags_int, and the polymorphic
boxed pair. The set_boxed mli doc comment documents the boxed-only
precondition (passing a non-boxed gir_record causes type confusion at
GValue finalization).
Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a)
Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 3)
Chris Armstrong
Stage 4 of milestone-2-signals Phase 1a. enum_code.ml now produces a companion <ns>_enums.ml alongside the existing <ns>_enums.mli, with a <lower>_of_int / <lower>_to_int pair for every enum and bitfield. Enum decoders pattern-match on int (raising Failure on out-of-range) and encoders go variant-tag -> int. Bitfield decoders accumulate via bit tests; encoders fold via lor. Duplicate-int members are dedup'd in _of_int (first wins) and emitted in full in _to_int. Refactored 6 duplicated digit-prefix/uppercase blocks into variant_name_of_member helper. Added 5 AST-validating tests in enums_tests.ml (file existence, _of_int, _to_int, bitfield _of_int, mli val declarations). Generator emission only — Stage 5 removes the dune (modules_without_implementation) clause and regenerates the bindings. Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a) Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 4) Chris Armstrong
…dules_without_implementation Stage 5 of milestone-2-signals Phase 1a. Runs the full bindings regeneration so the generator change from Stage 4 produces the new <ns>_enums.ml companion files. 8 namespaces gain a generated .ml (cairo, gdk, gdkpixbuf, gio, graphene, gsk, gtk, pango); pangocairo has no enums and is unchanged. Each touched namespace's dune file drops its (modules_without_implementation <ns>_enums) clause now that the .ml exists. gtk/dune retains the 6 cross-namespace .mli-only entries (gdkpixbuf_enums, gsk_enums, graphene_enums, gdk_enums, gobject_enums, pango_enums) whose .ml lives in the owning namespace's directory. enum_code.ml gains a small follow-up patch: the .ml emission now re-declares the polymorphic-variant types alongside the function bodies (OCaml does not auto-import types from the .mli). Without this the regenerated code did not compile. gsk_decls.h picks up three additional cross-namespace #include lines (gdkpixbuf, pango, pangocairo) that the dependency walker emits in batch mode. Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a) Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 5) Chris Armstrong
…64 wrappers Stage 6 of milestone-2-signals Phase 1a. Adds ocgtk/tests/gtk/test_signal_value_enum_flags.ml with 10 cases covering the new wrappers committed across Stages 2-5: enum/flags round-trip via decoder, flags-empty-list, variant string round-trip, int64 max_int round-trip, decoder failure on out-of-range int, decoder property-test for orientation, bitfield decoder property-test for modifiertype across six subsets, type-mismatch (Invalid_argument) on get_flags_int against a G_TYPE_INT GValue, and a compile-time witness for NO_MODIFIER_MASK. Includes a small C stub (gtk_orientation_get_type wrapper) so the test can construct a typed GtkOrientation GValue without going through GIR-bound construction. Test placed under ocgtk/tests/gtk/ because it needs gtk_test_helpers.require_gtk for the gtk_init guard. Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1a) Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 6) Chris Armstrong
…e dispatchers
Stage 7 of milestone-2-signals Phase 1b. Pure module
gir_gen/lib/generate/signal_marshaller.{ml,mli} exports `classify`,
which takes a generation_context and a gir_type and returns either
Supported { ocaml_type; getter_expr; setter_expr } or Unsupported reason.
Same-namespace types resolve to bare module paths (e.g.
"Gtk_enums.orientation"); cross-namespace types prefix the wrapper
package (e.g. "Ocgtk_gdk.Gdk_enums.modifiertype"). The setter/getter
expressions reference the GValue primitives committed in Phase 1a.
Adds 15 unit-test cases in
gir_gen/test/class_generation/signal_marshaller_tests.ml covering all 6
primitives, same- and cross-NS enum/bitfield/GObject, GLib.Variant,
GArray-as-Unsupported, boxed-as-Unsupported, callback-as-Unsupported,
and the void-return path. Test wiring lands in Stage 8.
Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1b)
Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 7)
Chris Armstrong
Stage 8 of milestone-2-signals Phase 1b. Adds signal_marshaller_tests
to gir_gen/test/dune and registers ("Signal Marshaller",
Signal_marshaller_tests.tests) in the Layer 2 section of
test_gir_gen.ml. The 15 unit tests committed in Stage 7 now execute
alongside the existing suite; total gir_gen test count rises from 349
to 364.
Spec: gir_gen/docs/plans/milestone-2-signals.md (Phase 1b)
Plan: .claude/plans/milestone-2-signals-phase1.json (Stage 8)
Chris Armstrong
…d follow-ups Adds PR link, final baseline (build/test counts), and 6 follow-up entries (D1-D6) capturing review-flagged items that did not block Phase 1a/1b completion: missing G_VALUE_HOLDS_BOXED guard on the set_boxed wrapper, optional consistency guards on set_int64/set_variant, the absent boxed runtime test, a stylistic Stdlib.compare in test_signal_value_enum_flags.ml, doc-drift in milestone-2-signals.md about the C-symbol count, and the NO_MODIFIER_MASK round-trip limitation. Chris Armstrong
…und-trip test Adds the missing G_VALUE_HOLDS_INT64 / G_VALUE_HOLDS_VARIANT / G_VALUE_HOLDS_BOXED checks to the matching three setter wrappers in ml_gobject.c, raising Invalid_argument on type mismatch. The set_boxed guard in particular closes the type-confusion window the .mli already warned about and is required before the signal emitter starts using it. Adds a runtime round-trip test in ocgtk/tests/gtk/test_signal_value_enum_flags.ml exercising the boxed GValue path end-to-end with a GdkRectangle: construct -> set_boxed -> get_boxed -> assert field equality via gdk_rectangle_equal. Two small C stubs (gdk_rectangle_get_type registration, four-int constructor) support the test without any production-side changes. Chris Armstrong
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ml_gobject.c(get/setpairs forint64,variant,enum_int,flags_int,boxed) and exposes them as OCaml externals inGobject.Value. The boxed pair reuses the existinggir_record_custom_opsinfrastructure withg_boxed_copyownership;set_boxed's precondition (boxed-only GIR record) is documented in the.mli.Gobject.Value.set_object Noneno-op that left the GValue uninitialised —Nonenow correctly stores NULL via a newml_g_value_set_object_nullprimitive. Includes regression test.<ns>_enums.mlcompanion to the existing<ns>_enums.mlifor every namespace with at least one enum or bitfield. Pure-OCaml<lower>_of_int/<lower>_to_intare produced for each declared type. Regenerates bindings for all 8 affected namespaces (cairo, gdk, gdkpixbuf, gio, graphene, gsk, gtk, pango); drops(modules_without_implementation <ns>_enums)from the corresponding dune files. PangoCairo has no enums and is unchanged. Thegtk/duneretains its 6 cross-namespace shadow.mli-only entries.gir_gen/lib/generate/signal_marshaller.ml— a pure module mapping aTypes.gir_typeto either a marshalling spec (OCaml type string + getter/setter expressions, with same-NS / cross-NS qualification) or anUnsupportedreason. This is consumed by upcoming signal-emission generator work.ocgtk/tests/gtk/test_signal_value_enum_flags.ml(10 cases covering every new wrapper) andgir_gen/test/class_generation/signal_marshaller_tests.ml(15 cases covering primitives, same/cross-namespace enum/bitfield/GObject, GLib.Variant, and unsupported branches). gir_gen test count rises 344 → 364; ocgtk gains 11 cases.Test plan
dune build --root .exits 0 cleanlydune test --root . gir_gen/— 364 tests passxvfb-run dune test --root . ocgtk/— 329 cases pass, 0 failuresgrep -c ml_g_value_get_ ocgtk/src/*/generated/*.c— zero per-type C symbols addedGobject.Value.set_object Noneregression test reads backNoneKnown limitations / follow-ups (not addressed in this PR)
ml_g_value_set_boxeddoes not yet runtime-checkG_VALUE_HOLDS_BOXED. No production caller invokes it yet; needs to land before the signal emitter starts generatingset_boxedcalls.get_boxed/set_boxedround-trip — the build verifies linkage but a runtime case will be added when the signal emitter exercises the path.ml_g_value_set_int64andml_g_value_set_variantlackG_VALUE_HOLDS_*guards on the setter side, mirroring the pre-existing peer style forset_int/set_uint/etc.Gdk_enums.modifiertype'sNO_MODIFIER_MASK(value 0) is not round-trippable through bitfield decoders becauseflags land 0 <> 0is permanently false. This is faithful to the pre-existing C generator behaviour.chris-armstrong