Repository navigation
forms, table: pair reflected keys and values through one shared helper - #909
Merged
Merged
Conversation
Yaraslaut
force-pushed
the
fix/907-meta-member-pairing
branch
from
October 10, 2026 10:08
b9fc524 to
c75a0b0
Compare
forms::detail::forEachNamedMember named member I with glz::reflect<A>::keys[I] but read the value at get<I>(glz::to_tie(action)). to_tie follows declaration order and ignores glz::meta, so under a reordered or partial meta one member's value was handed to another member's name: a hidden member offered under a public key, or a key bound to a value of a different type. reconcileDeclaredPrecision walked the same tie and could miss a Quantity the meta lists. The table engine carried its own copy of the correct pairing (memberAt). Promote it to morph::detail::reflectedMember<I>(object) in include/morph/detail/reflected_member.hpp: the declaration-order tie for a type glaze reflects by itself, the meta's I-th value otherwise. forms (forEachNamedMember, reconcileDeclaredPrecision) and table (cellReaders, reflectedColumns) now both read through it. Closes #907 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
fix/907-meta-member-pairing
branch
from
October 10, 2026 10:36
c75a0b0 to
d35b784
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Closes #907
The defect
forms::detail::forEachNamedMembernamed memberIwithglz::reflect<A>::keys[I]but read the value atget<I>(glz::to_tie(action)).to_tiefollows declaration order and ignoresglz::meta. Under a reordered or partial meta, one member's value was handed to another member's name. That could offer a hidden member under a public key, or bind a key to a value of a different type.reconcileDeclaredPrecisionwalked the same tie, so it could skip aQuantitythat the meta lists.The shared helper
The table engine already had the correct pairing as a private copy (
table::detail::memberAt). Two copies of one defect is the "missing framework seam" case, so this PR promotes that pairing to one helper instead of writing a second fix:morph::detail::reflectedMember<I>(object)ininclude/morph/detail/reflected_member.hpp, next tofixed_string.hpp, the other cross-module detail utility. It reads the declaration-order tie for a type glaze reflects by itself, and the meta'sI-th value otherwise. It returns a reference into the object,constwhen the object isconst. A computed meta entry yields whateverglz::get_memberyields for it.Call sites:
forms:forEachNamedMember, so every caller inherits the fix:memberWireName/describe<>(), schema derivation, views' column walk, and the session walks.reconcileDeclaredPrecisionalso uses the helper directly. Under a meta it rounds only listed members, and it skips a computed (by-value) entry because there is no stored member behind it.table:cellReadersandreflectedColumns.memberAtis deleted.Spec:
docs/spec/forms/forms.md(rows forforEachNamedMemberandreconcileDeclaredPrecision). The table spec already describes reading through the meta, so its contract is unchanged.Verification (measured, clang-debug, Homebrew clang 23, pinned glaze
719ef1be)New tests:
tests/test_forms_meta_members.cpp: partial meta (each key gets its own member, and the hiddeninternalNoteis never offered), reordered meta (meta order, and writes reach the named member), plain aggregate (declaration order),memberWireNameunder reordered and partial metas, and the helper's reference/constness types.tests/test_quantity_forms.cpp:ReconcileDeclaredPrecision::FollowsAPartialMeta.Results:
morph_tests1845 test cases, all pass (the one "failed as expected" is a pre-existing[!shouldfail]).morph_table_testsall pass (26071 assertions in 125 test cases).Mutation 1: forms reverted to master's
forms.hpp, new tests kept. Excerpt:The two
[reflection]cases that still pass under the mutation are the plain-aggregate walk and the helper's own type checks, which do not go through the forms code. Restored, all pass.Mutation 2: helper forced to the
to_tiepath (if constexpr (true)). This shows the table really reads through the shared helper:Restored, all pass.
Not verified:
glz::meta(crm, lims, ledger) were configured withMORPH_BUILD_LADDER=ON. The final rebuild was stopped at 87/113 because the machine ran low on memory; it had no compile errors up to that point. Their full builds and tests are left to CI.-Wdocumentation, and Doxygen were not run locally.Review notes
glz::metaentry that wraps or computes a value (glz::quoted_num, a lambda) now reaches forms visitors as glaze's wrapper or value, not the underlying member, because that is what glaze serialises under the key. No forms action in tree uses such an entry; the onlyglz::customisRational's value meta, which is not an object meta.reconcileDeclaredPrecisionexplicitly skips by-value entries so that every registered action still compiles.T&, so a temporary cannot bind and leave a dangling reference.🤖 Generated with Claude Code