Repository navigation
feat(tir): export the impls table on PackageInterface (BEP-066 slice 6a, PR 2) - #4327
Merged
antoniosarosi merged 2 commits intoAug 7, 2026
Merged
Conversation
…6a, PR 2)
Add PackageInterface.impls: Vec<ExportedImpl> — the loc-free blob-side
twin of the impl_data substrate, so a source-less dependency can later
contribute its impls to matching, membership, and coherence (the R2
fails-open hole). Derivation only: nothing reads the new rows, the
ImplLoc enumerators are untouched, and diagnostics/bytecode stay
byte-identical (full baml_tests suite green, snapshots unchanged).
Schema (package_interface.rs):
- ExportedImpl { interface, for_ty_pattern, generic_params,
param_bounds, associated_types, field_links, origin, methods }.
Patterns are Ty over the impl's rigid ParamTy TypeVars (the currency
match_ty_patterns keys on — deliberately not TyTemplate); the
interface head carries args + realized assoc bindings (explicit pins
plus filled defaults), canonicalized by Interface::new's by-name sort.
- ExportedImplOrigin mirrors InterfaceImplOrigin (diagnostic metadata
ONLY — must not drive resolution/dispatch/coherence).
- ExportedImplMethod { name, sig }: a STRUCTURAL identity — the owner is
the enclosing row (interface head + for-ty) — never MIR's
{iface}$for${target} source-text naming, which the consumer PR
reconstructs from the pair. No spans anywhere (Salsa span-free rule).
Derivation (fold_package_interface -> exported_impls): enumerate
package_impl_locs, read impl_data verbatim; malformed impls
(Err(ImplDataError)) are skipped exactly as every resolution consumer
skips them — their diagnostics stay owned by impl_data/check.rs. Rows
sort by their borsh encoding: a canonical total order independent of
file enumeration order (two-database determinism).
Method signatures lower exactly as the conformance checker lowers the
override side (validate_impl_signatures): impl + method generics in
scope, method bounds joining the bounds map (the
resolve_interface_method_spec shape), and Self realized through
realize_with_symbolic_self (now pub(crate)) — rigid Self bounded by the
implemented interface, Self -> for_ty_pattern substituted last — NOT
function_signature_ty, which lowers free-impl methods without a Self
binding (Ty::Error mentions). Throws pairs the declared clause with the
callable_throws oracle, as every exported function does.
Tests (baml_tests compiler2_tir::package_interface): in-body impl with
field links + override rows, out-of-body impl realizing Self in
receiver/param/return positions, generic bounded impl
(implement<T extends Anchor> Pair<T,T> for Box<T>), assoc pins + filled
Self.Item[] default staying a symbolic projection on the receiver,
blanket implement<T> I for T, stdlib spot-check
(baml.ops implement Equals for int), borsh round-trip and two-database
determinism now covering impls. B-694 bytecode_cache suites stay green
(sentinel literal gains the new field).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe package interface now exports canonical, Borsh-serializable implementation rows with resolved types, bounds, associated types, field links, origins, and method metadata. Lowering realizes symbolic ChangesPackage implementation exports
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PackageFolding
participant InterfaceResolution
participant PackageInterface
PackageFolding->>InterfaceResolution: Read implementation locations and data
InterfaceResolution->>PackageInterface: Lower implementation rows and methods
PackageInterface->>PackageInterface: Sort rows by Borsh encoding
PackageInterface-->>PackageFolding: Return folded package interface
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks passed✅ 7 passed
Generated by |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…lice 6a, PR 3) (#4329) **BEP-066 slice-6a stack, PR 3 of 5** — chained on #4327. The first consumer PR: a package mounted as a serialized `PackageInterface` blob (no source in the DB) now fully resolves in **type positions**; calls land in PR 4 (reserved diagnostic E0158 until then). ## The mount seam - `MountedPackages` Salsa input (`alias → borsh(PackageInterface)`), present-from-construction like the B-694 seeds; served by a `package_interface` arm below the untouched stdlib seed; corrupt blobs fall through to ordinary unresolved diagnostics. - Dependency policy: user packages depend on every mounted name; mounts see only the stdlib. Reserved names (`baml`, `user`, …) are ignored outright — a blob can never shadow the stdlib or the user package. ## Consumption - `resolve_type_in` returns `Own(Definition) | Foreign(&ExportedType)`; `lower_path`'s Foreign arm mirrors Own exactly (arity, binding validation, existential default-filling by substitution of the pre-lowered symbolic-`Self` defaults — no double-fill, completeness checks). - Fact queries split seed-or-source: aliases, enum variants (+ exhaustiveness), class fields/methods, assoc bounds/defaults/name-sets, `interface_requires` via the exported pre-flattened closure (fail-closed), UFCS/value-reference typing. - Impl substrate: blob `ExportedImpl` rows materialize into native `ImplData` (`ImplInterfaceTarget::{Source,Mounted}`, `ImplRef::{Loc,Mounted}` — the loc-free `ResolvedImpl` twin); all five enumerators + coherence run unchanged over one `package_impls` seam; blob impls are pre-checked and never re-validated; user impls of mounted interfaces get full conformance checking. Coherence attributes primary-only at the user's impl with the dep side rendered structurally. - Schema refinement: `ExportedType::Interface.methods` → `required_methods`/`default_methods` (E0113 coverage is undecidable from a merged list). - PR 1's interface-row guards became the mounted gate: rows resolve iff mounted; source-backed dep resolution is byte-identical (tested both ways). ## Proof 12 new mounted-consumption tests (class/interface/enum/alias/impl/bound/negative shapes + coherence overlap + E0158 both-ways) · full baml_tests exit 0 (lib 1511/0) · tir 324/0 · project 82/0 · bytecode_cache 43/0 · **zero snapshot churn** · fmt/clippy clean on diff. ## For PR 4 (queued) `MemberResolution::External` + MIR symbolic callees, blob-impl method dispatch + structural `{iface}$for${target}` reconstruction, the `$stream`-rows policy, and the conservative gaps listed in the commit message. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for resolving types, interfaces, implementations, aliases, enum variants, fields, and methods from mounted dependency packages. - Added package discovery and serialized interface loading for source-less dependencies. - Added validation for mounted package coherence, generics, associated types, interface requirements, and default methods. - **Bug Fixes** - Prevented crashes when source locations are unavailable. - Added diagnostic `E0158` for unsupported calls into mounted packages. - Improved conflict diagnostics for mounted dependency implementations. - **Tests** - Expanded coverage for mounted packages, conformance, aliases, exhaustiveness, and error handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Antonio Sarosi <[email protected]>
This was referenced Aug 7, 2026
This branch was previously deployed
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.
BEP-066 slice-6a stack, PR 2 of 5 — chained on #4326. Derivation only, zero behavior change (
implsis written, never read; consumers land in PR 3/4).What
PackageInterface.impls: Vec<ExportedImpl>— everything a source-less consumer needs for impl matching and conformance withoutImplLocs: interface head (args + realized assoc bindings),for_ty_patternasTyover the impl's rigidParamTys, generic params + bounds, assoc rows in declaration order, field links, origin (doc-carried as diagnostic-metadata-only), and per-method rows with structural identity (owner = the row's interface+for-ty pair; MIR's source-text-derived{iface}$for${target}naming deliberately not replicated — the consumer PR reconstructs it structurally).Key choices
validate_impl_signatureslowers the impl side (InterfaceMethodSpecspine +realize_with_symbolic_self,Self → for_tysubstituted last) — NOTfunction_signature_ty, which lowers free-impl methods without aSelfbinding (its own documented deficiency).ImplDataError; diagnostics stay owned byimpl_data.Proof
implement Equals for intspot-check (other: Self→int).realize_with_symbolic_selfvisibility (pub(crate)) + a test-sentinel literal.Summary by CodeRabbit
New Features
Bug Fixes
Tests