feat(stylo_taffy): eliminate panic paths, add tracing feature + test suite#443
Open
nixpt wants to merge 1 commit into
Open
feat(stylo_taffy): eliminate panic paths, add tracing feature + test suite#443nixpt wants to merge 1 commit into
nixpt wants to merge 1 commit into
Conversation
…suite
Hardens stylo_taffy by replacing all `unreachable!()` panic paths with
safe fallbacks to `AUTO` / `DEFAULT`, with optional debug logging when a
new `tracing` feature is enabled. Adds a placeholder unit-test suite for
the conversion functions.
This work was developed downstream in nixpt/bliss-engine (a fork of this
repo for the Exosphere ecosystem) where rendering panics surfaced in
real-world CSS that uses anchor positioning, sizing keywords, or fit-
content() across `min-content` / `max-content` / `stretch` /
`-webkit-fill-available`. Brought back here so blitz users see the same
robustness improvements.
## Changes
### `convert.rs` — panic-elimination (+186/-50 net)
- All `unreachable!()` paths replaced with safe fallbacks to taffy's
AUTO / DEFAULT. Anchor positioning functions, sizing keywords, and
other CSS values that taffy doesn't model now degrade silently
instead of panicking. The previous comment "Anchor positioning will
be flagged off for time being" suggests the unreachable!() were known
TODOs; this PR closes them by making the fallback the documented
behavior.
- Integer overflow protection for grid line numbers and repeat counts
using `i16::try_from` + bounds-checked fallbacks (was silent overflow
→ panic via `as i16` casts).
- New `log_fallback!()` macro logs each fallback at `tracing::debug!`
level when the `tracing` feature is enabled; the no-tracing arm
consumes its args via `let _ = (&$value, &$to);` to silence
`unused_variables` warnings at call sites.
- Improved `# Safety` doc on `length_percentage()`.
### `lib.rs` — documentation (+31/0)
Expanded module-level docs with Features / Limitations / Safety
sections covering the new `tracing` feature, the deliberate CSS-feature
fallback list, and the `unsafe` rationale in
`convert::length_percentage()`. Hooks in `#[cfg(test)] mod tests;`.
### `tests.rs` — placeholder unit tests (new file, +209)
35 scaffolded tests organized by conversion path (dimension, position,
overflow, grid, flexbox). Tests don't yet exercise full Stylo fixtures
but lock in test-module organization so future hardening has a hook to
extend from. Doc-comment on the file explains the placeholder status.
### `Cargo.toml` — `tracing` optional feature (+2/0)
`tracing = { workspace = true, optional = true }` (the workspace
already declares `tracing = "0.1.40"` at line 169) plus a `tracing =
["dep:tracing"]` feature gate. Default features unchanged; opting into
tracing only adds the new debug-log behavior.
## Test plan
- [x] `cargo check -p stylo_taffy` — clean (default features)
- [x] `cargo check -p stylo_taffy --features tracing` — clean
- [x] `cargo check -p stylo_taffy --all-features` — clean
- [x] `cargo test -p stylo_taffy --all-features` — 35 passed, 0 failed
## Downstream context
The same hardening shipped to nixpt/bliss-engine as PR #1 (merged
2026-05-17) and is now consumed in production by Exosphere's super-
surfer browser + voyager-tui + crush-ide. No regressions surfaced
downstream during integration testing.
This was referenced May 18, 2026
nixpt
added a commit
to nixpt/bliss-engine
that referenced
this pull request
May 18, 2026
…d_variables (#6) The `log_fallback!()` macro has two arms — one for `feature = "tracing"` that calls `tracing::debug!()` (which consumes both args), one for the no-tracing build that was an empty `{}`. The empty arm leaves the caller's bindings unused, producing a `unused_variables` warning at call sites like: unsupported => { log_fallback!(&format!("display:{:?}", unsupported), "DEFAULT"); taffy::Display::DEFAULT } The `unsupported` binding becomes unused in no-tracing builds, even though it IS used in the macro call (semantically). Compiler can't see through the macro expansion to know that. Fix: change the no-tracing arm to `let _ = (&$value, &$to);` which consumes both args at the call site without emitting any code at runtime. The `&` ensures we don't move out of the caller's bindings. Backport of the same fix from DioxusLabs#443 (the upstream of upstream — same macro, same problem, same fix). PR-D of 4 in the s190 cascade arc. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
(at RustWeek this week, will review this later in the week or next week) |
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
2 participants
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
Hardens
stylo_taffyby replacing allunreachable!()panic paths with safe fallbacks toAUTO/DEFAULT, adds optional debug logging via a newtracingfeature, and ships a placeholder unit-test suite for the conversion functions.Originally developed downstream in nixpt/bliss-engine (a blitz fork for the Exosphere ecosystem) where rendering panics surfaced in real-world CSS using anchor positioning, sizing keywords (
min-content/max-content/fit-content/stretch/-webkit-fill-available), and similar features taffy doesn't currently model. The existing comment inconvert.rs"Anchor positioning will be flagged off for time being"suggested theunreachable!()arms were known TODOs; this PR closes them by making the safe fallback the documented behavior.Changes
convert.rs— panic-elimination (+186/-50)unreachable!()paths replaced with safe fallbacks. Anchor positioning, sizing keywords, and other unsupported CSS values degrade silently toAUTO/DEFAULTinstead of panicking.i16::try_from+ bounds-checked fallbacks (was silent overflow → panic viaas i16casts).log_fallback!()macro logs each fallback attracing::debug!level when thetracingfeature is enabled. The no-tracing arm consumes its args vialet _ = (&$value, &$to);so callers don't tripunused_variableswarnings on their bindings.# Safetydoc onlength_percentage().lib.rs— module documentation (+31)Expanded module-level docs covering Features / Limitations / Safety, the deliberate CSS-feature fallback list, the
unsaferationale inconvert::length_percentage(), and hooks in#[cfg(test)] mod tests;.tests.rs— placeholder test suite (new file, +209)35 scaffolded tests organized by conversion path (dimension, position, overflow, grid, flexbox). The tests don't yet exercise full Stylo fixtures but lock in test-module organization so future hardening has a hook to extend from. Doc-comment on the file explains the placeholder status.
Cargo.toml—tracingoptional feature (+2)tracing = { workspace = true, optional = true }(workspace already declarestracing = \"0.1.40\"at line 169) plustracing = [\"dep:tracing\"]feature gate. Default features unchanged; opting into tracing only adds the new debug-log behavior.Test plan
cargo check -p stylo_taffyclean (default features)cargo check -p stylo_taffy --features tracingcleancargo check -p stylo_taffy --all-featurescleancargo test -p stylo_taffy --all-features— 35 passed, 0 failedDownstream context
Same hardening shipped to nixpt/bliss-engine as nixpt/bliss-engine#1 (merged 2026-05-17), now consumed in production by Exosphere's super-surfer + voyager-tui + crush-ide. No regressions surfaced downstream during integration testing.
🤖 Generated with Claude Code