From b637c11386a5e0d3c28506543ec5150e82c12d97 Mon Sep 17 00:00:00 2001 From: Brian Searls Date: Mon, 20 Jul 2026 21:09:06 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(compile-clean):=20dissolve=20the=20?= =?UTF-8?q?=C2=A73=20loader-closure=20fork=20=E2=80=94=20gate=20loader=20r?= =?UTF-8?q?uns=20both=20closures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit load_compile_clean_entry_sources (the affected-set compile-clean gate loader) ran only extend_with_reference_closure, while the witness loader load_sources_for_entry_with_pool runs BOTH that and extend_with_bare_reference_closure in a fixpoint. Both functions' doc-comments claimed to be the ONE closure authority; the code was forked. The service-name and bare-name provider edges (gcp.STS -> dag/extdeps/cloud/gcp/sts.dag) live ONLY in the bare closure, so an affected entry reaching a provider purely through a service call or bare name (dag/gunbc/auth/patterns.dag -> gcp.STS.Exchange, zero imports) dropped that provider from the scoped compile set and its names went unresolved. This surfaced non-locally when #6937's import strip made patterns.dag affected. Fix: the gate loader runs the same both-closure fixpoint as the witness loader, via a primary-precedence MultiEntryIndex (build_multi_entry_index_primary_precedence). Proven by execution (scoped_gate_loader_pulls_bare_referenced_providers, #[ignore]d heavyweight): RED control = ref-only closure of patterns.dag has 3 unresolved-type hard diags and no sts.dag; fixed = sts.dag pulled, 0 hard diags. Whole-tree compile-clean after the change = 0 hard diagnostics (no cross-tree poisoning). Memory note (cc governor lane): this brings the gate path to parity with the witness loader's both-closure cost — NOT a new cost class, but more resolution work on the gate path; relevant to the #6848 floor-memory regression being measured in parallel. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/v1/stage0/src/cli_run.rs | 154 +++++++++++++++++++++++++++++++---- 1 file changed, 140 insertions(+), 14 deletions(-) diff --git a/src/v1/stage0/src/cli_run.rs b/src/v1/stage0/src/cli_run.rs index 60438dbec9f..d49b1656b89 100644 --- a/src/v1/stage0/src/cli_run.rs +++ b/src/v1/stage0/src/cli_run.rs @@ -1586,10 +1586,11 @@ fn index_source_root_into_module_index( fn load_compile_clean_entry_sources( source_roots: &[String], - index: &ModuleSourceIndex, - facts: &ModuleGraphFactsLive, + mei: &MultiEntryIndex, entry_path_filter: Option<&std::collections::HashSet>, ) -> Result>, String> { + let index = &mei.source_files; + let facts = &mei.module_graph_facts; let first_root = std::path::Path::new(&source_roots[0]); let mut entry_files = Vec::new(); if first_root.is_dir() { @@ -1630,9 +1631,29 @@ fn load_compile_clean_entry_sources( sources.push(Rc::new(v1_compiler_compile::SourceFile { path, content })); } } - let mut sources = extend_with_reference_closure(sources, index, facts)?; - sources.sort_by(|a, b| a.path.cmp(&b.path)); - sources.dedup_by(|a, b| a.path == b.path); + // BOTH closures to a joint fixpoint — the SAME authority the witness loader + // `load_sources_for_entry_with_pool` runs (a §3 dissolution; both functions' + // doc-comments claimed to be "the ONE closure authority" while the code was + // forked: this one ran only `extend_with_reference_closure`). The + // module-path reference closure and the bare/service-name reference closure + // pull disjoint edges — the service-name → provider edge (`gcp.STS` → + // dag/extdeps/cloud/gcp/sts.dag) lives ONLY in the bare closure — so an + // affected entry that reaches a provider purely through a bare name or a + // service call (patterns.dag → `gcp.STS.Exchange`, no import) dropped that + // provider from the scoped compile set and its names went unresolved + // (proven: ARM1 ref-only = 3 unresolved-type diags on patterns.dag's closure; + // +bare = 0). Iterate because each newly-pulled module can itself carry + // either edge kind. + loop { + let before = sources.len(); + sources = extend_with_bare_reference_closure(sources, mei)?; + sources = extend_with_reference_closure(sources, index, facts)?; + sources.sort_by(|a, b| a.path.cmp(&b.path)); + sources.dedup_by(|a, b| a.path == b.path); + if sources.len() == before { + break; + } + } Ok(sources) } @@ -2345,9 +2366,8 @@ fn witness_layer_roots_compile_clean_sources_for_plan( CompileCleanScopePlan::WholeTree => { eprintln!("compile-clean scope: whole-tree entry closure (witness_layer_roots)"); let roots = witness_layer_roots(); - let index = build_module_index_primary_precedence(&roots); - let facts = build_module_graph_facts_live(&roots); - load_compile_clean_entry_sources(&roots, &index, &facts, None).map(|mut sources| { + let mei = build_multi_entry_index_primary_precedence(&roots); + load_compile_clean_entry_sources(&roots, &mei, None).map(|mut sources| { append_test_floor_compile_clean_inject(&mut sources); Some(sources) }) @@ -2363,9 +2383,8 @@ fn witness_layer_roots_compile_clean_sources_for_plan( .map(|p| workspace_relative_repo_path(p)) .collect(); let roots = witness_layer_roots(); - let index = build_module_index_primary_precedence(&roots); - let facts = build_module_graph_facts_live(&roots); - load_compile_clean_entry_sources(&roots, &index, &facts, Some(&filter)).map(Some) + let mei = build_multi_entry_index_primary_precedence(&roots); + load_compile_clean_entry_sources(&roots, &mei, Some(&filter)).map(Some) } } } @@ -4532,9 +4551,8 @@ fn entry_source_from_index_or_disk( fn load_sources( source_roots: &[String], ) -> Result>, String> { - let index = build_module_index(source_roots); - let facts = build_module_graph_facts_live(source_roots); - load_compile_clean_entry_sources(source_roots, &index, &facts, None) + let mei = build_multi_entry_index(source_roots); + load_compile_clean_entry_sources(source_roots, &mei, None) } #[derive(Debug, Clone, PartialEq, Eq)] @@ -4761,6 +4779,20 @@ pub fn build_multi_entry_index(source_roots: &[String]) -> MultiEntryIndex { new_multi_entry_index_shell(build_module_index(source_roots), source_roots, None) } +/// Primary-precedence `MultiEntryIndex` — the index shape the compile-clean gate +/// uses (`--dependency-pool-index primary-precedence`: root[0] wins, later roots +/// fill only absent modules). Needed so `load_compile_clean_entry_sources` can run +/// the SAME both-closure fixpoint as the witness loader (`extend_with_bare_reference_closure` +/// requires the `MultiEntryIndex` for its per-tree bare census), dissolving the §3 +/// closure-authority fork the two loaders' doc-comments each falsely claimed to be single. +fn build_multi_entry_index_primary_precedence(source_roots: &[String]) -> MultiEntryIndex { + new_multi_entry_index_shell( + build_module_index_primary_precedence(source_roots), + source_roots, + None, + ) +} + pub fn build_multi_entry_index_with_shared_caches( source_roots: &[String], cross_worker_store: Arc>, @@ -22498,3 +22530,97 @@ mod sigs_env_flat_parents { ); } } + +#[cfg(test)] +mod compile_clean_loader_closure_fork_regression { + // Regression for the §3 closure-authority fork dissolved 2026-07-20: the + // compile-clean gate loader `load_compile_clean_entry_sources` ran ONLY + // `extend_with_reference_closure` (module-path refs), while the witness loader + // `load_sources_for_entry_with_pool` ran BOTH that and + // `extend_with_bare_reference_closure`. The service-name → provider edge + // (`gcp.STS` → dag/extdeps/cloud/gcp/sts.dag) and bare-name provider pulls + // live ONLY in the bare closure, so an affected entry reaching a provider + // purely through a service call or bare name (dag/gunbc/auth/patterns.dag → + // `gcp.STS.Exchange`, zero imports) dropped that provider from the scoped + // compile set and its names went unresolved. This surfaced non-locally when + // #6937's import strip made patterns.dag affected. Fix = the gate loader runs + // the same both-closure fixpoint as the witness loader. + // + // Heavyweight (builds the whole-tree index; ~20s) and chdir-global, so it is + // #[ignore]d like `witness_layer_roots_compile_clean_check` — run explicitly: + // cargo test -p v1-compiler --lib compile_clean_loader_closure_fork \ + // -- --ignored --nocapture --test-threads=1 + use super::*; + + fn hard_diags(sources: &[Rc]) -> Vec { + v1_compiler_compile::compile_sources( + Rc::new(sources.to_vec().into()), + crate::v1_compiler_artifact::RenderTarget::Dag, + ) + .diagnostics + .iter() + .filter(|d| compile_clean_diagnostic_is_hard(d)) + .map(|d| crate::v1_std_core::diagnostic_to_message(d.diagnostic.clone())) + .collect() + } + + #[test] + #[ignore = "heavyweight (whole-tree index) + chdir-global; run explicitly"] + fn scoped_gate_loader_pulls_bare_referenced_providers() { + std::env::set_current_dir(workspace_root()).expect("chdir workspace root"); + let roots = witness_layer_roots(); + let mei = build_multi_entry_index_primary_precedence(&roots); + + let patterns_rel = "dag/gunbc/auth/patterns.dag".to_string(); + let filter: std::collections::HashSet = [patterns_rel].into_iter().collect(); + + // RED control: the OLD ref-only behavior, replicated inline. Resolve the + // scoped entry + ONLY the module-path reference closure — no bare closure. + // The service-only provider must be ABSENT and the closure must red. + let entry_source = + entry_source_from_index_or_disk(&mei.source_files, "dag/gunbc/auth/patterns.dag") + .expect("entry source"); + let mut ref_only = resolve_transitively( + vec![entry_source.clone()], + &mei.source_files, + &mei.module_graph_facts, + ) + .expect("resolve"); + if !ref_only.iter().any(|s| s.path.contains("patterns.dag")) { + ref_only.push(entry_source); + } + let ref_only = + extend_with_reference_closure(ref_only, &mei.source_files, &mei.module_graph_facts) + .expect("ref closure"); + let sts_ref_only = ref_only + .iter() + .any(|s| s.path.contains("cloud/gcp/sts.dag")); + let diags_ref_only = hard_diags(&ref_only); + assert!( + !sts_ref_only, + "RED control broken: ref-only closure unexpectedly already contains sts.dag" + ); + assert!( + !diags_ref_only.is_empty(), + "RED control broken: ref-only scoped closure of patterns.dag must produce unresolved-name \ + hard diagnostics (the fork this test guards). Got zero — the discriminating red is gone." + ); + + // The FIX: the real gate loader, now running BOTH closures. The bare + // closure must pull the service/bare-referenced providers and the scoped + // compile must be clean. + let fixed = load_compile_clean_entry_sources(&roots, &mei, Some(&filter)) + .expect("fixed scoped load"); + let sts_fixed = fixed.iter().any(|s| s.path.contains("cloud/gcp/sts.dag")); + let diags_fixed = hard_diags(&fixed); + assert!( + sts_fixed, + "fix regressed: the both-closure gate loader must pull the service provider sts.dag \ + into patterns.dag's scoped closure" + ); + assert!( + diags_fixed.is_empty(), + "fix regressed: patterns.dag scoped compile must be clean under the both-closure loader, got: {diags_fixed:?}" + ); + } +} From 9f7c411c26d554e6830722a786ed8a548fe58ab0 Mon Sep 17 00:00:00 2001 From: Brian Searls Date: Mon, 20 Jul 2026 21:28:07 +0000 Subject: [PATCH 2/2] WIP: 6848 cleanup --- src/v1/stage0/src/cli_run.rs | 68 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/v1/stage0/src/cli_run.rs b/src/v1/stage0/src/cli_run.rs index d49b1656b89..749c76bc383 100644 --- a/src/v1/stage0/src/cli_run.rs +++ b/src/v1/stage0/src/cli_run.rs @@ -1631,30 +1631,15 @@ fn load_compile_clean_entry_sources( sources.push(Rc::new(v1_compiler_compile::SourceFile { path, content })); } } - // BOTH closures to a joint fixpoint — the SAME authority the witness loader - // `load_sources_for_entry_with_pool` runs (a §3 dissolution; both functions' - // doc-comments claimed to be "the ONE closure authority" while the code was - // forked: this one ran only `extend_with_reference_closure`). The - // module-path reference closure and the bare/service-name reference closure - // pull disjoint edges — the service-name → provider edge (`gcp.STS` → - // dag/extdeps/cloud/gcp/sts.dag) lives ONLY in the bare closure — so an - // affected entry that reaches a provider purely through a bare name or a - // service call (patterns.dag → `gcp.STS.Exchange`, no import) dropped that - // provider from the scoped compile set and its names went unresolved - // (proven: ARM1 ref-only = 3 unresolved-type diags on patterns.dag's closure; - // +bare = 0). Iterate because each newly-pulled module can itself carry - // either edge kind. - loop { - let before = sources.len(); - sources = extend_with_bare_reference_closure(sources, mei)?; - sources = extend_with_reference_closure(sources, index, facts)?; - sources.sort_by(|a, b| a.path.cmp(&b.path)); - sources.dedup_by(|a, b| a.path == b.path); - if sources.len() == before { - break; - } - } - Ok(sources) + // BOTH closures to a joint fixpoint via the ONE shared authority the witness + // loader `load_sources_for_entry_with_pool` also calls (a §3 dissolution: this + // gate loader previously ran ONLY `extend_with_reference_closure`, so an + // affected entry reaching a provider purely through a bare name or a service + // call — patterns.dag → `gcp.STS.Exchange`, no import — dropped that provider, + // since the service-name → provider edge `gcp.STS` → dag/extdeps/cloud/gcp/sts.dag + // lives ONLY in the bare closure, and its names went unresolved. Proven: ARM1 + // ref-only = 3 unresolved-type diags on patterns.dag's closure; +bare = 0). + extend_sources_to_both_closure_fixpoint(sources, mei) } /// Reference-derived dependency closure (namespace Rule-1 interim). A qualified @@ -4444,20 +4429,23 @@ fn extend_with_bare_reference_closure( /// versa. This is the loader the witness paths use; the raw-pair /// `load_sources_for_entry_with_index` stays the dotted-only base for callers /// without a pool index. -fn load_sources_for_entry_with_pool( - index: &MultiEntryIndex, - entry_path: &str, +/// The ONE closure-extension authority: run the bare/service-name and the +/// module-path reference closures to a joint fixpoint (each newly-pulled module +/// can carry either edge kind). Both source loaders — the per-entry witness +/// loader and the affected-set compile-clean gate loader — call this, so the +/// single-authority claim in `extend_with_reference_closure`'s doc-comment is +/// true by construction rather than by two functions happening to hold identical +/// loop bodies (the §2 duplicate that dissolving the §3 fork would otherwise +/// have left behind). +fn extend_sources_to_both_closure_fixpoint( + mut sources: Vec>, + mei: &MultiEntryIndex, ) -> Result>, String> { - let mut sources = load_sources_for_entry_with_index( - &index.source_files, - &index.module_graph_facts, - entry_path, - )?; loop { let before = sources.len(); - sources = extend_with_bare_reference_closure(sources, index)?; + sources = extend_with_bare_reference_closure(sources, mei)?; sources = - extend_with_reference_closure(sources, &index.source_files, &index.module_graph_facts)?; + extend_with_reference_closure(sources, &mei.source_files, &mei.module_graph_facts)?; sources.sort_by(|a, b| a.path.cmp(&b.path)); sources.dedup_by(|a, b| a.path == b.path); if sources.len() == before { @@ -4467,6 +4455,18 @@ fn load_sources_for_entry_with_pool( Ok(sources) } +fn load_sources_for_entry_with_pool( + index: &MultiEntryIndex, + entry_path: &str, +) -> Result>, String> { + let sources = load_sources_for_entry_with_index( + &index.source_files, + &index.module_graph_facts, + entry_path, + )?; + extend_sources_to_both_closure_fixpoint(sources, index) +} + fn load_sources_for_entry_with_index( index: &ModuleSourceIndex, facts: &ModuleGraphFactsLive,