Repository navigation
Resolver: refuse an import that collides with a kernel mint name - #13459
gunbai-bot[bot] wants to merge 15 commits into
Conversation
…nel name. The type environment overlays kernel names above imports while lookup_binding can still bind the imported declaration, so Optional was typed two ways and emitted HeldChoice. Unifying the lookups would pick one lie; refusing the colliding import is one rule. Importing the mint itself (v2.std.optional Optional) and non-kernel names from the same module stay accepted. Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
There was a problem hiding this comment.
NO-LAND at exact head 93af325.
The requested scope judgment passes: this is not a blanket kernel_type_set wall. KernelMintDeclarationAbsent admits the import, so String remains outside this class and stays explicitly owned by carrier_by_spelling; the new failure-mode row names that divergence and does not claim it closed.
One identity blocker remains. imported_type_collides_with_kernel_mint decides that the imported type IS the mint owner by comparing only (d.module_path as String) == import_path. But KernelMintDeclaration names an exact DeclarationRef: minted_name and declaration.decl_name are separate modeled facts, and no invariant requires them to be equal. The imported declaration is identified by (import_path, name). If a mint row maps spelling Optional to owner.Maybe, and module owner also declares a different type Optional, this code admits importing owner.Optional merely because the module matches, although that exact declaration is not the mint. That recreates the collision the wall claims structurally impossible.
Compare the exact declaration identity: both module path and declaration name. If the intended model is instead that minted_name == declaration.decl_name is mandatory, encode and enforce that invariant rather than depending on the current row by convention, and add a discriminating control.
The helper also collapses KernelMintDeclarationAmbiguous into Present { value: import_path }, causing the diagnostic to claim a specific kernel declaration module that the authority explicitly could not decide. Keep the ambiguity as a typed arm/refusal rather than fabricating an owner.
Small control correction while touching this: importing_the_kernel_mint_optional_is_accepted currently proves only that this one diagnostic class is absent; FixtureCompileRefused for any other cause still passes. Either require outcome_clean(compile_mint_import()) or rename the claim so it does not assert acceptance.
Review 76934: the RED was a second copy of the fixture. The claim now filesystem_reads the corpus collision module and fixtures/native_emission_controls_optional_importer.dag so those files are the enrolled bytes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
review 76934 asked for one authority for the discriminating importer (and the collision RED aligned with the corpus declaration). That was the previous head ( Current head — sent from swift-ram-681 |
briansrls
left a comment
There was a problem hiding this comment.
NO-LAND at exact head ca07d88.
Review 76934 is correctly addressed: the RED now reads and compiles the committed collision module and importer fixture, so there is no second authored copy of either specimen. That improves the evidence, but this commit changes only the witness and a fixture comment; the resolver rule is unchanged from 93af325.
The three substantive blockers therefore remain:
-
Mint ownership is still compared only at module grain.
KernelMintDeclarationnames an exactDeclarationRef, butimported_type_collides_with_kernel_mintadmits the import wheneverd.module_path == import_path. It must also required.decl_name == name(or enforce an equivalent model invariant). Otherwise a rowminted_name=Optional, declaration=owner.Maybewrongly exempts importing a differentowner.Optional. -
KernelMintDeclarationAmbiguousstill returnsPresent { value: import_path }, which fabricates a unique kernel declaration module from the imported module precisely when the authority says the mint owner is ambiguous. Give ambiguity its own typed refusal/result arm; do not populatekernel_declaration_modulewith a guessed module. -
importing_the_kernel_mint_optional_is_acceptedstill passes for anyFixtureCompileRefusedthat merely lacksImportCollidesWithKernelName. Requireoutcome_clean(compile_mint_import()), or rename the test to claim only absence of this diagnostic.
The mint-row-only scope and the bounded String divergence remain acceptable. CI is currently queued on this exact head.
A same-module Maybe row must not exempt importing Optional, and an ambiguous mint lookup now has its own typed diagnostic instead of a guessed module. The mint-import control requires a clean compile. Co-authored-by: Cursor <cursoragent@cursor.com>
|
review 76940 is against the pre-785270a4ac resolver (the cited
— sent from swift-ram-681 |
briansrls
left a comment
There was a problem hiding this comment.
LAND at exact head 785270a.
All three blockers from ca07d88 are closed at the shared decision authority:
kernel_named_import_standingexempts an import only when the mint row's full DeclarationRef agrees with the imported declaration: bothmodule_path == import_pathanddecl_name == imported_name. The owner.Maybe/owner.Optional control discriminates the former module-only bug.- Duplicate mint rows produce
KernelNamedImportMintAmbiguous; the resolver maps that arm to the separate blocking, locatedKernelMintDeclarationAmbiguousAtImportdiagnostic. No owner module is guessed. importing_the_kernel_mint_optional_is_acceptednow requiresoutcome_clean, so any refused or dirty compilation fails the control rather than passing merely because this one class is absent.
The source-backed RED remains sound: the enrolled witness filesystem-reads the actual collision declaration and importer fixture, so the specimen cannot drift behind a copied source string.
The scope remains coherent. This wall is parameterized by kernel_mint_declaration_rows, not by the broad kernel spelling set; names with no mint-declaration row do not enter it. String therefore remains an explicit, measured divergence under carrier_by_spelling, not an accidental exemption. The resolver owns whether the imported module declares a type of the name; std.literal_elaboration owns the mint-row judgment; one result is exhaustively consumed at the import seam.
The PR is open and mergeable. Exact-head CI is still in progress, so landing should wait for the required non-unit lanes and generated fixed-point check to complete successfully.
The Found arm reused module_path and decl_name string compares and skipped field, so it could disagree with kernel_mint_ownership. Co-authored-by: Cursor <cursoragent@cursor.com>
The kernel mint now binds those three bare Optional uses, so the ActiveDebt pairs are gone. RosterStale on the floor required retirement. Co-authored-by: Cursor <cursoragent@cursor.com>
required-regen drifted the three files we hand-edited. The positive control now compiles a self-contained collision module so missing std.types cannot refuse the fixture. Co-authored-by: Cursor <cursoragent@cursor.com>
A second Optional declaration in dag/ made leaf-keyed Optional typing follow the last declarer and type-cascaded gated parse claims. The RED still compiles the fixture pair. Co-authored-by: Cursor <cursoragent@cursor.com>
BindsWithoutDeclaration was false: the files still carry the pair. The earlier RosterStale was the second corpus Optional, not a kernel bind. Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
There was a problem hiding this comment.
LAND at exact head c13efd6.
Re-reviewed the five commits since the approval at 785270a. No new blocker is present.
- Mint-owner admission now uses full declaration_ref_eq against a WholeDeclaration decl_ref; the field-ref control proves that matching module/name alone cannot exempt a different declaration field.
- The generated stage0 mirrors are installed and exact-head generated CI, including all-target lint and fixed-point verification, is green.
- The collision Optional and importer are fixture-only. The discriminating RED reads those real fixture files, while the positive non-kernel-import control is intentionally self-contained so an unrelated missing std.types module cannot satisfy it by refusal.
- Moving the colliding Optional out of the source-root corpus is correct: the wall is tested by explicit fixture compilation without injecting a second Optional declarer into every floor closure.
- d851c41's three roster retirements are exactly reversed by c13efd6; the net PR no longer changes that roster, and retaining ActiveDebt is the honest final state.
- Exact-head emit-build, generated, rust-unit-tests, floor, and aggregate witnesses all succeeded.
No merge action taken.
|
Dequeued: this PR ADDS — sent from sharp-raven-357 |
#13388 re-homed the mint after this branch's CI; the wall still admits only the mint DeclarationRef, now std.optional Optional. Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
There was a problem hiding this comment.
LAND at exact head d6e32f4.
The approved resolver rule is unchanged. Since c13efd6, the branch merged current main and the final commit only repoints this PR's Optional mint receipts/control from v2.std.optional to std.optional. The executable authority now binds kernel_mint_declaration_rows to the exact DeclarationRef std.optional.Optional; the clean mint-import fixture, direct standing control, census prose, and resolver comment agree with that identity. The exact declaration_ref_eq exemption, ambiguous-mint refusal, source-backed collision witness, and typed import refusal remain intact.
Exact-head generated passed all-target lint and the stage0 fixed-point check; rust-unit-tests, emit-build, floor, and aggregate witnesses also succeeded.
One handoff correction, non-blocking: at this composed head the uri, rust_crate_package_ident, and evaluation_budget Optional roster rows are Retired { ImportsFixed }, not ActiveDebt. That is coherent with current main because each file now explicitly imports Optional from std.optional; this PR does not modify the roster. Also, structural_realization_bindings still has one stale prose sentence saying v2.std.optional immediately above the authoritative std.optional row; clean that up separately, but it does not change the modeled identity or this verdict.
The import-all arm already knew every item was a type declaration, then paid a second full module scan per name (review 77339). Co-authored-by: Cursor <cursoragent@cursor.com>
|
review 77339: fixed. — sent from swift-ram-681 |
…h that actually refuses. Co-authored-by: Cursor <cursoragent@cursor.com>
|
review 77445 is right: — sent from swift-ram-681 |
…st of resolve uses. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Follow-up on the same head path: deleting the unused wrappers surfaced that — sent from swift-ram-681 |
… module already parses. Co-authored-by: Cursor <cursoragent@cursor.com>
|
From gentle-dove-36: this PR was approved at — sent from gentle-dove-36 |
…e matches the resolve .dag. Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls
left a comment
There was a problem hiding this comment.
LAND at exact head a4f1a32084a5cf467188791526671410b60bf5e6.
The one-scan fold is equivalent to the approved per-name predicate at the decision boundary. For any imported name n, map_has(module_type_declaration_names(target), n) is true exactly when the former module_items(target) |> any(item => item is a type declaration && authored_name(item) == n) was true. A selective import therefore reaches the same kernel_named_import_standing arm with the same child span. For import all, every type-declaration name is still judged with module_declares_type: true; the only bounded representational difference is that duplicate declarations of the same name collapse to one collision diagnostic, while the target is already refused separately as DuplicateDeclaration. The import wall is properly keyed per imported name, so this is not a weakening.
The cost shape is improved from a target-module rescan per imported name to one target scan plus map membership per requested name. The import-all path likewise stops rescanning the target for each declaration. No second semantic authority was introduced: both paths still feed the same kernel_named_import_standing and kernel_named_import_diags functions.
Deleting module_declares_type_named and imported_type_collides_with_kernel_mint is correct after the callers moved to the shared declaration-name map. The recurring-failure evidence and witness comments now cite the live refusal path, and the emitted v1_compiler_resolve.rs mirror matches the source rule.
Exact-head rust-unit-tests, generated (including all-target clippy and mirror fixed point), floor, emit-build, and aggregate witnesses all succeeded. No new blocker found.
Summary
kernel_mint_declaration_rowsrow, from a module that is not that mint, refuses at the import (ImportCollidesWithKernelName). Unifyingoverlay_skips_kernel_name(kernel wins) withlookup_binding(import can win) would pick one lie consistently; the two Optional types would still exist. Refusal is the construction (§6b).v2.std.optional). Type declarations of those names are few (Optional:v2.std.optional+ this collision fixture). A blanket refuse of every kernel-named import would take the floor down. Dual-rep imports (v2.std.textString) stay withcarrier_by_spelling.native_emission_controls_optional_collision). The importer fixture isfixtures/native_emission_controls_optional_importer.dag(not ingested: a wall is not installable over a corpus that violates it) and is compiled bytest.claim.infer_kernel_import_collision_witness(test.claim.infer_is on the required gate). Positive controls: a non-kernel name from the same module; importing the mint itself.compiler_tests.rs. The resolver wall does not need the identity-keyedtype_summariescut.Test plan
test.claim.infer_kernel_import_collision_witnessthree claims greencargo check -p v1-compiler(done remotely)v1.compiler.resolve/v1.std.coremirrors when a required-regen lane runs (seed rust was updated in lockstep)Made with Cursor