Repository navigation
Variant collision diagnostic + factor algebra_templates_for_profile into std/algebra.dag - #279
Conversation
… collisions When two explicitly imported enums both define the same variant name, emit a warning diagnostic. The existing disambiguation logic (prefer the explicitly imported enum) still applies — this adds visibility into the collision without changing resolution behavior. Changes: - 00_core.dag: add VariantCollisionWarning variant to CompilerDiagnostic, wire into diagnostic_to_span, diagnostic_to_message, is_error_diagnostic - 04_infer.dag: add VariantFoldState type, refactor imported_variant_locals fold to track collision warnings, thread into ModuleContext diagnostics - v2_std_core.rs: mirror VariantCollisionWarning in stage0 Rust - v2_compiler_infer.rs: mirror VariantFoldState and collision detection Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
…nAlgebraCollectionProfile Move the AlgebraProfile->templates dispatch function from 04_types.dag to std/algebra.dag so the profile-to-template mapping lives with the data table and template functions it dispatches to. This reduces 04_types.dag to importing the dispatch function rather than defining it. Also adds BooleanAlgebraCollectionProfile (set algebra with union, intersect, diff, member) to stage0 Rust, aligning the Rust enum and data table with the .dag source. Changes: - dsl/std/algebra.dag: add algebra_templates_for_profile dispatcher - src/v2/04_types.dag: import algebra_templates_for_profile from std.algebra, remove local definition and per-profile fn imports - v2_compiler_infer_types.rs: add BooleanAlgebraCollectionProfile variant, boolean_algebra_collection_templates(), update KERNEL_ALGEBRA_PROFILE for Set/NonEmptySet, add arm in algebra_templates_for_profile Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
The test counted all diagnostics including warnings, but VariantCollisionWarning is a warning (not an error). Filter using is_error_diagnostic so only errors cause test failure. Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
Per INVARIANTS.md (escape hatch C-8): no warning severity in the compiler. All diagnostics are errors. - Rename OwnershipWarning → OwnershipViolation (00_core.dag, compile.dag, v2_std_core.rs, v2_compiler_compile.rs) - Rename VariantCollisionWarning → VariantCollision (00_core.dag, 04_infer.dag, v2_std_core.rs, v2_compiler_infer.rs) - Make is_error_diagnostic always return true (00_core.dag, v2_std_core.rs) - Remove warning/error severity branching in serialize_diagnostic (compile.dag, v2_compiler_compile.rs, main.rs) - Rename VariantFoldState.warnings → collision_errors - Revert pipeline.rs test to count all diagnostics (no filter) - Fix DSL collision: rename SymbolId variants Success/Failure/Warning/Info → StatusSuccess/StatusFailure/StatusWarning/StatusInfo to avoid collision with SemanticColor variants Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
briansrls
left a comment
There was a problem hiding this comment.
Review (2 violations, 2 good, 1 improve)
Invariant violations (2)
Modeling (good)
dsl/std/algebra.dagMoving the profile-to-template dispatch into std.algebra keeps the algebra witness as the single authority for which operations a kernel type inhabits, which is the right compositional layer.src/v2/04_types.dagDeleting the local algebra_templates_for_profile helper and importing the algebra authority removes a duplicate representation and improves harmony with the existing kernel_algebra_profile model.
Modeling (improve)
src/v2/00_core.dagVariantCollisionWarning models namespace ambiguity as advisory lint, but the domain fact is an unresolved ambiguity and would be more faithful as an error or explicit ambiguous-binding state.
The algebra helper extraction is a compositional improvement, but the new variant-collision handling is not fail-closed and introduces a fresh repeated-scan regression.
| }) | ||
| } else { vacc } | ||
| VariantFoldState { | ||
| locals: map_insert(vacc.locals, child.name, TypeBinding { name: child.name, resolved: binding.resolved }), |
There was a problem hiding this comment.
Invariant violation: When two explicitly imported enums share a variant name, this code still overwrites locals with one enum after only emitting VariantCollisionWarning, fabricating a winner for an ambiguous reference and violating No fallbacks that fabricate / Explicit boundary contracts.
| let curr_is_imported = resolved_imports |> any(imp => | ||
| imp.specific_names |> any(n => n == binding.resolved.name) | ||
| ) | ||
| let prev_is_imported = resolved_imports |> any(imp => |
There was a problem hiding this comment.
Invariant violation: The new prev_is_imported check rescans resolved_imports inside the inner variant fold instead of carrying an indexed import-membership fact forward, violating the Performance invariant's Facts Flow Forward rule.
|
Invariant review ( Violations (2):
Improve:
The algebra helper extraction is a compositional improvement, but the new variant-collision handling is not fail-closed and introduces a fresh repeated-scan regression. |
briansrls
left a comment
There was a problem hiding this comment.
Review (1 violations, 3 good, 3 improve)
Invariant violations (1)
Modeling (good)
src/v2/00_core.dagAddingVariantCollisionto the closedCompilerDiagnosticsum is the right core-layer model because ambiguity becomes a typed compiler fact instead of an ad-hoc message.src/v2/04_infer.dagVariantFoldStateis a coherent infer-local product type because it threads both chosen locals and accumulated collision diagnostics through one fold state.dsl/std/symbols.dagTheStatus*rename cleanly separates symbol identity fromSemanticColor, which is a faithful std-layer distinction.
Modeling (improve)
src/v2/00_core.dagNow that everyCompilerDiagnosticis effectively an error, the boundary would compose more cleanly if severity disappeared structurally instead of being represented by a degenerate classifier.src/v2/04_infer.dagA more compositional model would represent ambiguous variant lookup as its own binding shape instead of winner-plus-error, so later phases cannot consume a fabricated enum choice.dsl/std/symbols.dagIf the symbol vocabulary keeps growing, composestandard_symbolsfrom per-domain tables so the flat list stays derived instead of hand-expanded.
The PR improves the diagnostic and symbol models, but fail-closed handling is still incomplete and the previously flagged variant-collision issues remain unresolved.
| match decision { | ||
| SharedError { binding: binding, consumer_count: count, sites: sites } => | ||
| [make_error_node(diagnostic: OwnershipWarning { binding: binding, fn_name: proof.func_name, consumers: count, span: no_span() }, module_name: proof.func_name)] | ||
| [make_error_node(diagnostic: OwnershipViolation { binding: binding, fn_name: proof.func_name, consumers: count, span: no_span() }, module_name: proof.func_name)] |
There was a problem hiding this comment.
Invariant violation: Stage 6 now emits OwnershipViolation diagnostics here, but the pipeline still proceeds through artifact planning and emit without gating on ownership_diags, so a known error crosses the ownership→emit boundary in violation of the Early Detection invariant.
|
Invariant review ( Violations (1):
Improve:
The PR improves the diagnostic and symbol models, but fail-closed handling is still incomplete and the previously flagged variant-collision issues remain unresolved. |
INVARIANTS.md:
- Mark C-8 (Warning severity for semantic errors) as FIXED
- Strikethrough stale warning prose, add fix dates
- Update Root Cause C bullet and design-decision bullet
ROADMAP.md:
- Check off 'enrich_kernel_type calls .dag function in std/algebra.dag'
- Check off 'Convert per-profile field builders to .dag functions'
- Check off 'Fix Set/NonEmptySet profile' (Tier 2.5)
- Update ownership violation prose (no longer deferred)
docs/bootstrap-b-and-lane-work-plan.md:
- Add DONE status banners to sections 1 and 2
pipeline.rs:
- Fix stale comment ('not downgraded to warnings' → 'all diagnostics are errors')
Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
…ownership gate Violation 1 — No fallbacks that fabricate: When two explicitly imported enums collide on a variant name, the ambiguous binding is now REMOVED from locals instead of fabricating a winner. Later phases will see an unresolved variant and produce a proper error rather than silently using one enum's binding. Violation 2 — Facts Flow Forward: Pre-compute imported_enum_names as a Map<String, Bool> from resolved_imports BEFORE the variant fold. The inner fold performs O(1) map lookups instead of rescanning the import list for every variant of every enum. Violation 3 — Early Detection (ownership→emit boundary): Added a gate after ownership analysis in compile.dag and stage0 Rust. If ownership_diagnostics produces errors, the pipeline returns early with empty files instead of proceeding to artifact planning and emit. Co-authored-by: Brian Searls <briansrls@users.noreply.github.com>
Merged main (coercion design #278, variant collision #279, etc.) Resolved ROADMAP.md conflict: kept implemented TypeRendering struct, merged coercion dissolution notes. Reviewer fixes: - Removed orphaned Char type_map entry (chars() now returns List<Int>, no Char producer exists — reviewer correctly flagged inconsistency) - Updated ROADMAP E0c acceptance to reflect current implemented state Reviewer items deferred to follow-up (need design/modeling): - find_struct_by_fields heuristic → needs record literal inference to carry struct name (boundary sufficiency) - Fold refinement magic name checks → needs bidirectional inference - with() string-keyed branch → needs algebra operation registry - TypeRendering concern mixing → dissolves into coercion (M5) - Codepoint carrier modeling → algebra design work Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Variant collision diagnostic, algebra enrichment factoring, warning infrastructure removal, review violation fixes, and doc updates.
1. Variant collision diagnostic (Stream 1, violation #1)
Adds a
VariantCollisionerror diagnostic when two explicitly imported enums collide on a variant name. On collision, the ambiguous binding is removed from locals (fail-closed) — later phases see an unresolved variant rather than a fabricated winner.00_core.dag: newVariantCollision { variant, enum1, enum2, span }variant inCompilerDiagnostic04_infer.dag:VariantFoldStateaccumulator with pre-computedimported_enum_namesindex (FF-compliant); collision removes binding instead of overwritingv2_std_core.rs,v2_compiler_infer.rs)2. L1 Tier 2 — factor
enrich_kernel_typeto call.dagfunctions instd/algebra.dagMoves
algebra_templates_for_profilefrom04_types.dagintostd/algebra.dag, co-locating the dispatcher with the data table and per-profile template functions.dsl/std/algebra.dag: addsalgebra_templates_for_profilefunctionsrc/v2/04_types.dag: imports dispatcher fromstd.algebra, removes local definitionv2_compiler_infer_types.rs: addsBooleanAlgebraCollectionProfilevariant + templates, fixesSet/NonEmptySetprofile mapping3. Warning infrastructure removal (INVARIANTS.md C-8)
Per INVARIANTS.md: "no warning severity for semantic errors." Promotes all warnings to errors and eliminates the warning concept.
OwnershipWarning→OwnershipViolation;VariantCollisionWarning→VariantCollisionis_error_diagnostic→ always returnstrue"error"/"warning"severity branchingdsl/std/symbols.dag: renamedSymbolIdvariants to fix real collision withSemanticColor4. Review violation fixes (3 violations resolved)
Violation 1 — No fallbacks that fabricate: On variant collision between two explicitly imported enums, the binding is now removed from
localsinstead of fabricating a winner. Later phases see an unresolved variant.Violation 2 — Facts Flow Forward: Pre-computed
imported_enum_namesas aMap<String, Bool>fromresolved_importsbefore the variant fold. Inner fold performs O(1) map lookups instead of rescanning the import list per variant.Violation 3 — Early Detection (ownership→emit boundary): Added a gate after ownership analysis in
compile.dagand stage0 Rust. Ifownership_diagnosticsproduces errors, the pipeline returns early with empty files instead of proceeding to emit.5. Documentation updates
enrich_kernel_typeTier 2, per-profile field builders Tier 1, Set/NonEmptySet profile Tier 2.5Testing
cargo clippy --all-targets -- -D warnings— cleancargo test --workspace --exclude v2-compiler-tests— 12 passedcargo test -p v2-compiler-tests— 282 passedcargo test -p v2-compiler-tests full_dsl_compiles -- --ignored— passedcargo test -p v2-compiler-tests strict_compile_diagnostic_count -- --ignored— ratchet holds at 0