refactor(sugar): factory is the sole dispatch — reroute last decompose_* sites#2206
Conversation
…hrough the factory
The engine's term/composite dispatch already routes through the recursive
factory (build_term/build_composite over ordered recognizer registries), but
four collector/helper verdict-readers still constructed their node by calling
decompose_* inline. Move construction into the factory so it is the sole
dispatch:
- closure_method_terminal_effect -> sugar::factory::build_closure_adaptor
(reuses the existing closure_adaptor::recognize_composite single-shape walk).
- statement_position_terminal_effect -> sugar::factory::build_statement_position
(new statement_position::recognize entry; ctx-independent verdict).
- runtime_match_scrutinee_effect -> sugar::factory::build_match_scrutinee
(new ctx-free match_scrutinee::recognize over Expr::Match; the factory both
builds and reduces via the node's syntactic ctx-free reduction, returning
Outcome directly so the two ctx-less callsites route unchanged).
- impl_method_terminal_effect -> sugar::factory::build_item (new third tiny
ItemRecognizer registry + build_item, since an impl is a syn::Item, not an
Expr; impl_method::recognize is its sole entry).
Each construct still owns its own recognize+desugar; the factory gains only
registry entries + walk fns. Non-recognition now flows through the
UnsupportedSugar structural backstop (STRUCTURAL_BACKSTOP_REASON), which the
verdict-reading routers already map to None exactly as the old decompose_*
None bail -- behavior-preserving.
CallsiteSugar (the call-site inlining engine) is left as-is: it is already the
single consolidated inlining engine, does not implement the Sugar trait, and
its decompose/desugar contract (statement-sequence beta-reduction returning a
multi-entry CallsiteOutcome { Dug(InlineCommit) | Bail }) is incompatible with
the expr-in / Sugar-out factory dispatch -- routing it through build_* would be
a category error.
Gated on cargo test staying green: 533 passed / 0 failed (assertion_lift 302/0).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 19 minutes and 47 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
The four collector/helper verdict-readers that still called
decompose_*inline now construct through the factory: closure_adaptor / statement_position / match_scrutinee via thinbuild_*helpers, and impl_method via a new third registrybuild_item(ItemRecognizer = fn(&Item,&FactoryCtx)->Option<Box<dyn Sugar>>).decompose_*is no longer called anywhere inlib.rs(doc-comment mentions only). Each construct keeps owning its ownrecognize+desugar; the factory gained only registry entries + walk fns; no node branches on position.CallsiteSugar left untouched, by design: it's already the single consolidated inlining engine, but with a bespoke statement-sequence contract (β-reduces a helper body, re-runs
collect_assertion_entriesin scratch buffers, commits a multi-entryInlineCommit) — incompatible with the expr-in/Box<dyn Sugar>-out factory dispatch. Routing it throughbuild_*would be a category error.Behavior-preserving —
cargo test -p sugar-lift-rust-tests= 533 passed / 0 failed (incl. 302 assertion_lift). Not gated on coretests CID.🤖 Generated with Claude Code