Positions: retire updateITypePosition#1047
Open
nanavati wants to merge 5 commits into
Open
Conversation
Generic.everywhere rebuilds every node of the IModule whether changed or not; the transformation only rewrites ICon identifiers. An explicit traversal covers the same reachable ICon sites (heap references are leaves either way, since the cells sit behind IORefs), preserves untouched subtrees instead of reallocating them, and answers the old XXX about IAps recursion explicitly. bluetcl's find_vmodinfo becomes a direct query (ICVerilog is the only VModInfo carrier reachable in an IPackage), and with the last generic traversals gone, the Data/Typeable derivings on ISyntax and HeapData are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KHVhEzJpur1ZRjNjpzBVj
BSC's front end enforces one type constructor per qualified name, so a
qualified Id determines its (kind, sort) payload. Shipped comparison
code already relies on this invariant (TyCon Eq ignores the sort; ITCon
comparison is by Id alone), but nothing checked the compiler's
handwritten copies of Prelude tycon payloads against the Prelude truth.
Add a tconcheck utility that obtains a Prelude symbol table the
production way (BinUtil.readImports on an in-memory probe package, the
full all-defs signatures via BinUtil.replaceImportedSignatures exactly
as bsc's module-generation symtab does, then MakeSymTab.mkSymTab) and
verifies, entry by entry:
* every registered Type.hs TCon constant (new registry
Type.handwrittenTCons, 40 entries): Maybe Kind and TISort
* every registered ISyntaxUtil/IType ITCon constant (new registry
ISyntaxUtil.handwrittenITCons, 32 entries): IKind via IConv.iConvK
(newly exported) and TISort
* every StdPrel.preTypes row against the symbol table
* every StdPrel.preClasses tyConOf payload, both against the symbol
table and against its preTypes twin row (these are seeded through
independent paths in MakeSymTab with nothing else enforcing
agreement)
The check runs as part of the build (src/Libraries/Makefile), pointed
at the just-built libraries, so a Prelude edit that changes a payload
fails the build instead of silently drifting; tconcheck is built and
installed with the standard comp install for that purpose. A thin
testsuite wrapper (testsuite/bsc.misc/tcon_drift) re-checks the
installed tools.
The checker found nine pre-existing drifts, masked until now by the
payload-ignoring comparisons; they are documented and waived (loudly)
in tconcheck.knownDrift pending direction, e.g. Type.hs still calls
Int/UInt/File abstract although the Prelude declares them as data
types, ISyntaxUtil.itInout has kind # -> * although Inout is * -> *,
and itPrimPair's kind is left-nested because IKFun has no fixity
declaration.
In lifting the inline ITCon literals out of itPair/itList/itMaybe so
they could be registered (itPrimPair, itListCon, itMaybeCon), the
expressions are kept verbatim; no behavior change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVL9ornS6uf9hVxAuL7GhK
BSC's front end enforces one type constructor per qualified name, so a
qualified Id determines its (kind, sort) payload. This invariant was
already load-bearing in comparison code -- TyCon Eq ignored the sort,
and ITCon comparison in IType.cmpT is by Id alone -- but unstated, and
the TyCon instances mixed regimes: Eq compared kinds even for pairs
where the invariant makes that a dead tail, while remaining
kind-sensitive for the unqualified names where the invariant is
undefined.
Scope the TyCon comparisons by qualification:
* Eq: both-qualified pairs compare by Id (qualEq); the kind hedge is
kept only when either side is unqualified (resolution-era fuzz).
* Ord: lexicographic on (base, qual); within a (base, qual) bucket,
both-qualified ties are EQ without consulting the kinds, and the
unqualified bucket keeps the kind comparison. This remains a
lawful total order.
Document at the instances: the invariant and its qualified-only domain,
the deliberate non-transitivity of Eq via qualEq (pre-existing), the
Eq/Ord disagreement (pre-existing; containers key on Ord), and the
pointer to the tconcheck build step that verifies the handwritten
payload copies the by-Id comparisons rely on. Also upgrade the cmpT
comment in IType.hs: ITCon by-Id comparison is justified by the same
invariant (all ITCon Ids are qualified, since IType is born
post-resolution at IConv); the ITForAll binder-kind skip is a separate,
alpha-structural assumption, unchanged.
The only behavior delta is a both-qualified pair with the same name and
differing Maybe Kind fields. An audit of the pipeline found no
comparison site where a qualified Nothing-kinded TyCon (parse-era
trees, cTCon-built compiler-generated code) can meet its Just-kinded
twin (symtab/convCQType-era types): type-level Eq/Ord is only exercised
within a single kind regime, cross-regime checks use kind-agnostic
helpers (leftCon, mkInstId, Id comparisons), and no container is keyed
on Type/CType where mixed twins could coexist. The full testsuite and
a rebuild of all libraries with the changed compiler back this
empirically.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVL9ornS6uf9hVxAuL7GhK
Bring all nine drifted constants in line with the Prelude-derived truth
(the checker's DRIFT output was used as the spec), and empty the
tconcheck knownDrift waiver list; the waiver mechanism remains for
future incidents but ships empty.
* Type.hs tInt/tUInt: TIabstract -> TIdata (Prelude declares
'data Int n = Int (Bit n)' and likewise UInt)
* Type.hs tPrimPair (and StdPrel tiPair, used by itPrimPair):
SStruct -> SInterface [] (PrimPair is declared as an interface)
* Type.hs tActionValue: old struct sort -> TIdata [ActionValue]
('data ActionValue a = ActionValue ...')
* Type.hs tActionValue_: field names __value/__action ->
avValue_/avAction_ (new PreIds position variants added)
* Type.hs tFile: TIabstract -> TIdata [InvalidFile, MCD, FD]
(new PreStrings/PreIds constructor ids added)
* ISyntaxUtil itInout: kind # -> * (apparently copied from itInout_)
corrected to * -> *
* ISyntaxUtil itPrimPair: kind was left-nested, (* -> *) -> *,
because IKFun had no fixity declaration and defaulted to infixl 9;
IType.hs now declares infixr 8 `IKFun` (matching Kind's right-nested
Kfun and IConv.iConvK). Census of every backticked IKFun chain in
the tree: itPrimPair was the only unparenthesized multi-arrow chain;
all others are single-arrow or already explicitly parenthesized, so
the fixity change reparses nothing else.
* StdPrel tiBufferMode (used by itBufferMode): tiEnum -> tiData
(BlockBuffering carries a Maybe Integer argument, so BufferMode is
not an enum)
The corresponding position-parameterized variants (tIntAt,
tActionValueAt, tActionValue_At) are fixed to the same payloads.
A consumer audit found no reader of these payloads out of the constants
themselves: comparison sites use Eq/Ord (which compare by qualified Id)
or wildcard the payload fields in patterns; the payloads only travel
into generated ISyntax via iConvT, where they now agree with what
symtab-derived types already carried.
tconcheck now reports all 94 entries OK with zero waived.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVL9ornS6uf9hVxAuL7GhK
nanavati
force-pushed
the
itype-position-restamp
branch
from
July 14, 2026 18:15
cbc6d44 to
33718aa
Compare
Heap-ref (IRefT) types are no longer restamped with positions by updateIExprPosition/updateIExprPosition2: the poss field added in 7e17339 collects the stamped positions out of band, and once ITypes are hash-consed (next change) a restamping rebuild would re-canonicalize to the original node and drop the stamps anyway. The retirement alone moves no output. The position readers (getIExprPosition, getIExprPositionCross) now prefer the stamped positions on the heap ref, falling back to the type's own positions, so the poss field is no longer write-only. Reading poss regolds five tests: * bsc.evaluator/fileIO SetBufferingAfterClose: an outright improvement -- the S0085 file-handle error now blames the user's call site (SetBufferingAfterClose.bsv:10:21) instead of Prelude.bs internals. * bsc.misc/lambda_calculus lc-sysMethods/lc-sysStructs and bsc.misc/sal CTX_sysMethods/CTX_sysStructs: not position diffs but let-binding ordering flips -- these backends order parallel bindings by a position-influenced key, so the changed heap-ref positions flip the order of two bindings. Both orders are semantically equivalent and the new order is deterministic; the ordering's sensitivity to positions is a pre-existing fragility, not addressed here. Since the readers force poss, eqPtrs' redirected-ref rebuild now uses S.empty for that field instead of an internalError placeholder (the ref payload, which really is never touched, keeps its error). The five bsc.mcd ClockCheck outputs (GitHub issue B-Lang-org#863, pinned as .bad-expected) are byte-identical before and after this change: the evaluator-side position stamping was disconnected when -cross-info was removed in 2a0f45d, so those heap refs carry no useful poss entries yet. Restoring the desired use-site positions there is future work: re-enable stamping into poss at the evaluator sites; the readers here are ready to consume it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVL9ornS6uf9hVxAuL7GhK
nanavati
force-pushed
the
itype-position-restamp
branch
from
July 14, 2026 18:40
33718aa to
35719bd
Compare
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
1 participant
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.
Stacked on #1046 — only the head commit is new here.
What
Removes
updateITypePosition(ISyntaxXRef) — theIRefTcases ofupdateIExprPosition/updateIExprPosition2keep the reference's type unchanged (they already record the position inIRefT's out-of-bandpossset) — and flips theIRefTposition readers (getIExprPosition,getIExprPositionCross) to consumepossinstead of the type's embedded position.Why
A census of IType position consumers found the type-restamping machinery both expensive and mostly disconnected: every restamp deep-copies type spines to carry positions that Id equality ignores, it never touched types in
ILam/IApsand was shallow underITForAll, and thepossfield added toIRefTalready carries exactly this information out-of-band. With IType interning (stacked above), restamped rebuilds re-canonicalize and the stamps are dropped by construction — so the readers must move topossregardless; this PR makes that move independently testable.Observable effect (verified: the reader flip, not the retirement, causes all of it)
Five goldens move, regolded here with rationale:
bsc.evaluator/fileIO/SetBufferingAfterClose: the S0085 error now blames the user's call site (SetBufferingAfterClose.bsv:10:21) instead ofPrelude.bsinternals — a diagnostic improvement.bsc.misc/lambda_calculus(×2) andbsc.misc/sal(×2): let-binding ordering flips only — these backends order parallel bindings by a position-influenced key; both orders are semantically equivalent and the new order is deterministic. (The ordering fragility is pre-existing and noted, not addressed here.)The
bsc.mcdcross-clock (G0007) diagnostics are byte-identical: their restamp path was already disconnected upstream (see issue #863); the readers here are ready for when stamping is re-enabled.Validation
Full testsuite at this commit: 20276 PASS / 134 XFAIL with exactly the five regolded tests as the only movement vs the base branch (20281/0/134).
🤖 Generated with Claude Code
https://claude.ai/code/session_01LVL9ornS6uf9hVxAuL7GhK