Skip to content

Repair the coproduct-variant-name escape in 05_emit_rust already-filter (:3496): local_decl_names takes ONE name per top-level ITEM so variant names are never in it, a bare self-reference falls to the closure-global last-write-wins registry and a fabricated pub use is synthesised - fix module-local - #9523

Closed
briansrls wants to merge 3 commits into
mainfrom
session/deep-raven-866

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session deep-raven-866.
Pushing to session/deep-raven-866 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

Brian Searls and others added 3 commits August 27, 2026 14:52
…port candidates

reference_derived_use_lines already filters candidates against the names the
module declares, and its own note names the hazard: the bare-name registry is
last-write-wins across the closure, so a homonym in another module steals the
row and a self-colliding `pub use` is synthesized. The filter was written over
`items` -- one authored name per TOP-LEVEL item -- and a coproduct's variants
are CHILDREN of the item, so they never entered it. The guard was at the wrong
grain, not missing.

A bare self-reference to a local variant therefore fell through to
`map_get(registry, name)`, resolved to whichever module last wrote that global
name, passed export proof, and got a use-line for a symbol the module already
binds itself.

The variant set is read from each coproduct item's OWN children, never from
`emit_info.variant_to_enum` or `type_summaries`: both are keyed by bare name and
carry the same closure-global last-write-wins, so answering a module-local
question through them would reintroduce the defect one layer down.

MEASURED, two arms, each built from its own stage0 mirror, over the emitted
closure of `src/v2/compiler/00_compile.dag` (175 files):

  membership delta: exactly one file, `v2_std_node.rs`, which loses
    pub use crate::std_constructors::{Cardinality};
    use crate::std_constructors::Cardinality::*;
  `Cardinality` is a variant of v2.std.node's own `Connective`; the homonym is
  `std.constructors`' `type Cardinality = Required | Optional`. Nothing else in
  the 175 files changes membership -- the predicted set exactly.

  regen is a fixed point with the mirror installed
  (first_generation_equal=true, 136/136), so the v1 seed closure moves zero
  bytes.

  CONTROL, and it is a finding rather than a pass: run-to-run over the same
  binary, BOTH the unmodified and the modified compiler emit
  `v2_lens_enforcement_vocab.rs` and `v2_std_cross_tree_resolution.rs` with
  import blocks in varying ORDER (identical membership). That instability is
  present on main, is not caused by this change, and is why the arm comparison
  is stated at membership grain for those two files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/v1/05_emit_rust.dag
#	src/v1/stage0/src/v1_compiler_emit_rust.rs
@briansrls
briansrls marked this pull request as ready for review August 27, 2026 22:49
@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

No fix owed — the failure is on a superseded head, and the current head is already fully green.

run head build floor witnesses
33097895476 (17:20) 913d0730 success failure failure
33109314885 (19:37) 96000c2f5 success success success
33123971954 (22:48) 96000c2f5 in progress

The failing run is on 913d0730, which 96000c2f5 superseded. A complete run on the current head finished success across all three contexts at 19:37, including the witnesses aggregator that is the required context. The third run is a fresh execution of that same already-green head, so it re-measures rather than gates.

The alert reports "1 failing" because a failed check-run from the older head is still attached to the PR; that is a property of the check-run list, not of this head.

— sent from snappy-dove-250

@gunbai-bot gunbai-bot Bot mentioned this pull request Aug 28, 2026
6 tasks
@gunbai-bot

gunbai-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Closing as SUPERSEDED by #9461, not as rejected — the defect this PR describes is real and the repair it proposes is the one that shipped.

Measured, not inferred. src/v1/05_emit_rust.dag on current main (5f782fb6ef) already carries this fix:

  • local_coproduct_variant_names is declared at line 3611
  • local_variant_names is bound and folded into the already-filter at 3846–3847

and a direct file comparison between main and this branch produces zero differing lines mentioning variant_names or coproduct_variant — the two sides are identical on exactly the change this PR exists to make. #9461 ("Filter a module's own coproduct-variant names out of the emitter's import candidates", 58d10acc2e) landed it independently.

Every line that is unique to this branch is older content main has since revised — the pre-CallTargetIdentity import list, the pre-module_filename_collision_diagnostics guard block, and superseded comments. There is nothing here to salvage.

Why the diff looked like it still had content: git diff main...HEAD is three-dot and diffs from the merge base, so it reports this branch's additions against a base main has moved on from. It showed 23 changed .dag lines and none of them were unique to this branch. The two-dot/direct comparison is what settles supersession.

One note for whoever picks up the underlying class, since the fix landed without executing evidence: neither this PR nor #9461 carries a discriminating RED. A witness is authorable via the compile_dag_rust_emit_check fixture-provider pattern (the fourth argument is a must-NOT-contain slot, which is the right shape for asserting no fabricated pub use), but the fabrication only fires when the other module wins the closure-global last-write-wins bare-name registry. That makes the RED order-dependent: it could go permanently green when an unrelated module is added, which is worse than absent because it would be cited as coverage. A registry-order-independent way to express the collision is the missing capability, and it is the honest next-rung trigger for this class rather than something to paper over with a brittle test.

The branch's merge conflict was in the generated mirror v1_compiler_emit_rust.rs, where the merge driver refuses by design and leaves the ours side with no markers. That side carried Option<CallSemantics> where main now has Option<Rc<CallSemantics>>, which is a real E0308 — so the conflict could not have been resolved by picking a side, only by regeneration. Moot now that the PR is closed.

— sent from snappy-dove-250

@gunbai-bot gunbai-bot Bot closed this Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant