fix(types): bind package-local bare type refs to their own module identity (#2208)#2619
Closed
gertybotbot wants to merge 1 commit into
Closed
Conversation
…ntity (hew-lang#2208) A bare type reference inside a non-root module (e.g. a `Holder { w: Wrap }` field whose `Wrap` is declared in the same module) resolved through the `is_local` arm to the BARE `type_defs` key. That key is last-write-wins across modules that declare the same bare name — `pre_register_type_decl` documents the qualified alias as the authority — so when a sibling module declares its own `Wrap`, the field silently bound to whichever module registered `Wrap` last. This is observable in a correctly-disambiguated program (contradicting the issue's "unobservable" note): two package-local modules each with a divergent `Wrap` + `Holder { w: Wrap }` mis-resolve one holder's field to the other module's layout, surfacing as `no field `a_field` on type `Wrap` — help: b_field`, and the winner flips with import order. Fix: in the `is_local` arm, bind to the current module's OWN qualified identity (`{short}.{name}`) when that key exists, mirroring the sibling `published_bare_type_qualified` branch that already qualifies to dodge the same bare-key collapse. `pre_register_type_decl` always seeds the qualified key alongside the bare one, so non-colliding locals and root-module refs (`current_module_short() == None`) resolve exactly as before. Load-bearing regression `same_bare_name_member_field_binds_to_own_module_identity` asserts each holder's `w` field carries its own module's `Wrap` identity; it fails on revert (both collapse to the last-writer). Full hew-types (1665 lib + integration) and hew-hir suites green; fmt + clippy(--tests) clean.
Contributor
Contributor
|
Thanks for this — the #2208 collision fix is correct in intent and the observability correction is right. Review found one boundary hole: the own-qualified binding trips the visibility gate for same-module references to |
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.
Summary
Fixes #2208. A bare type reference inside a non-root module — e.g. a
Holder { w: Wrap }field whoseWrapis declared in the same package-local module — resolved through theis_localarm ofresolve_type_exprto the baretype_defskey. That key is last-write-wins across modules that declare the same bare name (pre_register_type_decldocuments the qualified alias as the authority), so when a sibling module declares its ownWrap, the field silently binds to whichever module registeredWraplast.Observability (correcting the issue note)
The issue notes this as "unobservable in correctly-disambiguated programs." It is in fact observable. Two package-local modules, each with a divergent
Wrapand aHolder { w: Wrap }, mis-resolve one holder's field to the other module's layout:…and the winner flips with import order (a program that reads
a.w.a_field/b.w.b_fieldcompiles or fails purely by which module'sWrapwas registered last).Fix
In the
is_localarm, bind to the current module's own qualified identity ({short}.{name}) when that key exists — mirroring the siblingpublished_bare_type_qualifiedbranch that already qualifies specifically to dodge this bare-key collapse.pre_register_type_declalways seeds the qualified key alongside the bare one, so:current_module_short() == None) are untouched,Boundary / neighbours checked
current_module == None): unchanged, still binds bare.published_bare_type_qualifiedandhandle_matchesarms below are untouched.Tests
New load-bearing regression
same_bare_name_member_field_binds_to_own_module_identityasserts each holder'swfield carries its own module'sWrapidentity (alpha.Wrap/beta.Wrap). It fails on revert (both collapse to the single last-writer identity), verified locally.cargo fmt+cargo clippy --tests: clean.42/helloand exits 0.