Repository navigation
Conversation
- finance-query's SentimentLabel (#[non_exhaustive]) failed to build: the glue's From<foreign> for mirrored conversion matched every known variant with no wildcard arm, which rustc rejects across a crate boundary once the source can gain variants without notice - add a catch-all arm to both foreign-to-mirrored conversions, unreachable for a binding built against the crate it was generated from, matching the error hierarchy's existing non_exhaustive handling - regression: soothfast-bind/src/pyo3/glue.rs unit test renders a plain enum's conversions and asserts the catch-all arm is present; it fails against the pre-fix body (confirmed by temporarily reverting plain_enum) and passes after Gates run: cargo fmt --all -- --check, cargo clippy -p soothfast-bind --all-targets -- -D warnings, cargo test -p soothfast-bind --lib (45 passed). Not run: workspace-wide clippy/test, shape_matrix --ignored (reproduces the same bug in the cabi-derived backends, out of scope for this Python-only brief; reported to the reviewer separately).
- cargo test -p soothfast-bind (not --lib) caught what the scoped run missed: tests/goldens/python/src/lib.rs still had Level's conversion without the wildcard arm 6432188 added - also trims the comment above arms() to two lines per review
- a non-plain enum always binds as an opaque handle (plan/mod.rs:270-
276); pyo3's handle_class never reads Class.variants field types, so
an UnmappedForeign gap on one reports a type that can never actually
cross on any backend
- resolve.rs recorded it anyway: the tuple-variant loop in
adt.rs::variant_fields called Resolver::resolve directly with no
visibility check at all, and finance-query's real Provider::Custom
(CustomId) shape hits exactly this
- route both tuple and named variant field resolution through
resolve_unreported, the same "resolved but never crosses" path
private struct fields already use
Regression: soothfast-bind/tests/surface.rs, a self-contained rustdoc
document (not the shared fixture, so no golden touches) with a tuple
variant over an unmapped foreign type. Confirmed failing pre-fix
(temporarily reverted adt.rs, reran):
UnmappedForeign { at: "shape::Stamped::At.0", path: "std::time::Duration" }
passes after.
Gates run: cargo fmt --all -- --check, cargo clippy -p soothfast-bind
--all-targets -- -D warnings, cargo test -p soothfast-bind (all
suites, 137 golden + 46 unit + 21 surface, all green, no golden
regen needed). Not run: workspace-wide clippy/test.
- adt.rs names a tuple field's position by its Rust index ("0", "1"),
legal as a tuple index but not as an identifier anywhere else;
finance-query's FactsByTaxonomy(HashMap<...>) renders `fn 0(&self)`
in the Python glue and `def 0(self)` in the stub, neither of which
compiles or parses
- every backend's own *_ident/rust_ident wraps the same shared
naming::escape, so a leading-digit rule there covers all of them at
once; Accessor.field itself has to stay the raw Rust access
expression (`self.0.0`), so the rename can only happen where the
target-language name is read out, never in the plan
Regression: tests/idioms.rs, an isolated tuple-struct document (not
the shared fixture) asserting the Python glue's getter is `fn _0`, the
stub's is `def _0`, and the repr shows `_0=`. Confirmed failing
pre-fix (temporarily reverted naming.rs): `fn 0(&self)`. Also a unit
test beside escape for a bare digit-leading name.
Gates run: cargo fmt --all -- --check, cargo clippy -p soothfast-bind
--all-targets -- -D warnings, cargo test -p soothfast-bind (all
suites green, 137 golden unchanged, no regen needed). Not run:
workspace-wide clippy/test.
- variant_named_fields duplicated named_fields except for the resolve call; named_fields now takes a reported flag, true for a struct's own fields (unmapped is a gap), false for an enum variant's (never a gap, since a data-carrying enum stays opaque regardless)
- out() only converted Ty::Class, Optional(Class), and List(Class); any other composition (Map(_, Class), Map(_, List(Class)), Optional(List(Class)), a Class inside a Tuple) fell through to `expr.to_string()` and returned the raw Rust value instead of the Python handle class. - Regression covers a Clone struct read through a map field, a map of lists, and a free fn returning a map, since all three go through the same out() call. Before the fix, the generated getters read `self.0.by_name.clone()` and `self.0.groups.clone()` verbatim, with no `Item(...)` wrapping at all.
- soothfast-demo/bindings/python's mirrored-enum conversions were stale: the last regen predates the non_exhaustive catch-all arm landing in the pyo3 backend, so `bind gen --check` was failing.
- d939b5a made a tuple struct's positional fields cross as _0, _1, ... in every language, since a bare index is not an identifier; the reference table never said so.
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gate |
Contributor
soothfast gatesoothfast-measuresoothfast-registrysoothfast-docssoothfast-specsoothfast-sdksoothfast-demosoothfast-reportsoothfast-sitebind: soothfast-demo |
This branch was successfully deployed
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.
What changed
Binding finance-query's Ticker surface to Python turned up four separate gaps in the pyo3 backend: a non_exhaustive plain enum had no catch-all arm and failed to compile against a real crate, a tuple struct's positional fields weren't legal Rust identifiers, a data-carrying enum's own payload got reported as a gap instead of crossing, and a map or map-of-lists holding an exported type came back as the raw Rust value instead of its Python handle class. Each fix carries its own regression, and the demo packages are regenerated so
bind gen --checkis clean again.Why
Each fix removes a gap or a compile failure the pyo3 backend hit while binding a real, non-synthetic crate's surface, not something found in the existing golden fixtures.
How was this tested
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace(all green, including the 137 golden tests, unchanged where the fix didn't touch their output)cargo test -p soothfast-bind --test python_smoke -- --ignored(builds and installs a real wheel)make checkcargo soothfast bind gen -p soothfast-demo --check(all twelve backends, exit 0)make gate BASE=masternot run: this branch targetsfeat/py-featuresand touches onlysoothfast-bindandsoothfast-demo, neither aBENCH_CRATE, so the merge-base gate doesn't apply to this diff.Checklist
make checkpasses (fmt, clippy-D warnings,cargo test --workspace)make gate BASE=masterpasses, or any intentional cost change is explained above (does not apply to this diff, see above)///doc commentsREADME.md,docs/,soothfast:bind/soothfast:claimmarkers) updated if behavior changed