P6.x: convert emitter::block_uses_emit to read the checker's own resolved Callee - #1203
Merged
Conversation
…lved Callee Closes the real inconsistency #1202's own review found and documented (in a code comment on unit_table_uses_emit, not silently): before this commit, unit_table_uses_emit (project.rs, converted in #1202) read the checker's resolved Callee::Capability{cap:"Events",op:"emit"}, while its per-handler twin block_uses_emit (deciding emit_service/emit_agent's own deps.__eventsDispatch parameter threading) still matched a bare Ident("Events") receiver by name -- the same syntactic approach both used before #1202, and both accepted as an "approximation" per this function's own prior doc comment (matching block_uses_send's sibling precedent). That approximation stopped being harmless the moment only one of the two checks became precise: a locally-declared type also named `Events` with its own static `emit` method (legal Bynk, resolves to Callee::Static, not Callee::Capability) made the two disagree -- unit_table_uses_emit correctly skips compose/fan-out generation, while block_uses_emit still threads a deps.__eventsDispatch parameter with nothing left to supply it. Confirmed with a real fixture: 1204_events_emit_shadowed_by_local_type failed `tsc --strict` under #1202 alone (found during that PR's own review, documented, deliberately not landed broken) and passes clean now that both checks agree. block_uses_emit's own callers all already had a TypedCommons/CheckedProgram in scope (emit_service/emit_agent take `commons`, the ir::lower callers take `program`), so this is purely a signature change plus call-site threading -- no new plumbing needed, unlike #1202's own project.rs-level fix. block_uses_send needs no matching conversion: a `~>` send is a real Statement::Send AST variant, not a method call that could be shadowed, so it was never approximate the way block_uses_emit's method-name match was. Correctness evidence: BYNK_BLESS=1 cargo test -p bynkc --test e2e bless_positive_fixtures touched zero existing files. `cargo test -p bynkc --test tsc_verify` (tsc --strict over every fixture's real emitted output) is clean, including the new fixture. ast_importers unaffected (still 8). Part of #1137. A further step toward project.rs's own slice 6 cleanup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
emitter::block_uses_emit— the per-handler twin decidingemit_service/emit_agent'sown
deps.__eventsDispatchparameter threading — now reads the checker's own resolvedCallee::Capability{cap:"Events",op:"emit"}instead of matching a bareIdent("Events")receiver byname.
not left silent.
unit_table_uses_emit, project-wide, andblock_uses_emit, per-handler) now fully agree on every input.The inconsistency this closes
Before #1202, both
unit_table_uses_emit(project.rs) andblock_uses_emit(emitter.rs) matched abare
Ident("Events")receiver by name — the same syntactic approach, both accepted as a deliberate"approximation" per
block_uses_emit's own prior doc comment (mirroringblock_uses_send's siblingprecedent). #1202 converted
unit_table_uses_emitto read the checker's own resolvedCallee, but leftblock_uses_emituntouched — its own review caught this and documented it explicitly rather thanpapering over it with a fixture that would have failed
tsc --strict.A locally-declared type also named
Eventswith its own staticemitmethod (legal Bynk, resolves toCallee::Static, notCallee::Capability) makes the two disagree:unit_table_uses_emitcorrectlyskips compose/fan-out generation, while
block_uses_emitstill threads adeps.__eventsDispatchparameter with nothing left to supply it — a real
tsctype error, not just cosmetic drift.Why this was mechanical, unlike #1202's own project.rs plumbing
Every one of
block_uses_emit's callers already had aTypedCommons/CheckedProgramin scope —emit_service/emit_agenttakecommons, their::lowercallers takeprogram. So this is purely asignature change (
block_uses_emit(b: &Block, callees: &HashMap<ExprId, Callee>) -> bool) pluscall-site threading — no new data-flow plumbing needed, unlike #1202's own
RunChecks::Checkedthreading.
block_uses_sendneeds no matching conversion: a~>send is a realStatement::SendASTvariant, not a method call that could be shadowed, so it was never approximate the way
block_uses_emit's method-name match was.Correctness evidence
BYNK_BLESS=1 cargo test -p bynkc --test e2e bless_positive_fixturestouched zero existing files.1204_events_emit_shadowed_by_local_type— the exact shape that failedtsc --strictunderP6.x: thread per-unit Callee data forward; convert unit_table_uses_emit and called_cross_context_services #1202 alone (found during that PR's own review, documented, deliberately not landed broken) — now
compiles and passes
tsc --strictclean.cargo test -p bynkc --test tsc_verify(realtsc --strictover every fixture's emitted output) is clean, including this one.
ast_importersunaffected (still 8).cargo build --workspace/cargo clippy --workspace --all-targets/cargo fmt --all -- --check/cargo test --workspaceall clean (164 test-result blocks, 0 failures, includingemitted_typescript_passes_tsc_strict).Done when
block_uses_emitreadsCallee, not a bare-Identname match.emitter.rs,emitter/emit.rs×7,ir/lower.rs×3).tsc --strict.unit_table_uses_emit's own doc comment updated — the "known follow-on" it named in P6.x: thread per-unit Callee data forward; convert unit_table_uses_emit and called_cross_context_services #1202 isresolved, not just left stale.
block_uses_sendconfirmed not needing the same fix, with the reasoning stated.ast_importersunaffected (still 8) — stated explicitly.design/pending/p6-block-uses-emit-callee.mdadded (patch level — internal only, no languagesurface change beyond fixing a real
tsc-failing edge case).Part of Phase 6 — The IR (spine) #1137 — a further step toward
project.rs's own slice 6 cleanup.