check: literal-defaulting cleanup follow-ups (post #9605)#9673
Open
jaredramirez wants to merge 16 commits into
Open
check: literal-defaulting cleanup follow-ups (post #9605)#9673jaredramirez wants to merge 16 commits into
jaredramirez wants to merge 16 commits into
Conversation
7ce0317 to
e4eb79e
Compare
Collaborator
😄 |
70d9be1 to
1a06357
Compare
unaryReceiverNeedsParens only guarded e_num_from_numeral; a unary operator over a negative e_typed_num_from_numeral re-emitted as --5.U8, which re-tokenizes as binary minus. Merge the two from-numeral arms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ia accessors Drop the redundant binop_negated/num_literal fields (re-derived from origin at 5 clone sites) in favor of binopNegated()/numeralInfo() accessors, matching types.Origin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The signature-footprint walk was copy-pasted in boundaryDefaultLeaksIntoSignature and runLiteralDefaultingRounds; these must agree or a literal could be partitioned non-interfering yet warned. One helper, one source of truth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract rangeNumeralDigitsFit and candidateSatisfiesRangeConstraints so the single-literal and group defaulting probes call one implementation of the precheck and phase-2 verify instead of hand-synced copies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p default The multi-driver else-arm of runLiteralDefaultingRounds re-implemented the before/commit/after warning bracketing that commitGatheredLiteral owns for the single case. Introduce commitGatheredGroup, symmetric to commitGatheredLiteral, so the else-arm shrinks to one call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the dangling doc comment for a deleted interpolation predicate and update from_numeral/from_quote references to the from_literal + LiteralKind representation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse six identical 9-arg ReportBuilder.init call sites into a helper and stop rebuilding the report builder per problem in assertTypeErrorMsgs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a comptime assertion pinning surface_origin_binop_offset to the unit tag count and a round-trip test over all SurfaceOrigin forms, so a future unit variant can't silently collide with the binop range. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move bench_probe_attempts/bench_probe_refuted from process-global pub vars to Check fields, and gate increments + their assertion test behind comptime std.debug.runtime_safety so release builds compile them out to a true no-op. Removes global mutable state and the hot-path cost in release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Kind The numeral>quote>interpolation precedence was replicated in varLiteralKind, flexLiteralDefaultKind, and numericDefaultPhaseForConstraints, synced only by a doc comment. One kernel on StaticDispatchConstraint; each site keeps its own surrounding logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RocEmitter kept a hand-synced parallel binding-power table (comment falsely claimed it mirrored the parser 'exactly'); a parser precedence change could silently make re-emitted operator expressions reparse to a different tree. Make the parser the single owner and map Binop.Op to its token to look up the one table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
defaultLiteralsAtGeneralizationBoundary re-scanned the whole module constraint list on every fixpoint round. Collect the .eql edges into a front-loaded reusable buffer once per call and iterate that instead, so the fixpoint costs rounds*eql_edges rather than rounds*all_constraints. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promote the 8 runLiteralDefaultingRounds buffers and 3 commitLiteralGroupDefault buffers to reused Check fields, matching the existing boundary_* front-loaded scratch. Each is cleared at the start of its step/use; the step-4 cascade (which can re-enter the function) reads none of them, so a shared field is safe under that reentrancy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promote the unifyMatchAltPatternBindings and payload-emptiness scratch buffers to reused Check fields. Each buffer is filled then consumed by a pure exhaustive.* call or non-recursive helper, so the owning function is never re-entered while the buffer is live. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
validateRecordRow/validateTagUnionRow recurse through nested record/tag types while their duplicate-name set is live, so a shared field would clobber the outer frame. Use a per-frame stackFallback (stack-backed, heap fallback on overflow), matching the arg_vars_sfa pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promote the external_pinnable AutoHashMap to a reused Check field, matching its already-front-loaded siblings (pinnable_vars, reported_dispatch_vars, pinnable_spine_visited) in the same one-shot ambiguity sweep. The platform-hosted-section maps stay local: that section only runs for platform modules (genuinely may-not-run) and its two maps own their keys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1a06357 to
53f6b1c
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
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.
Follow-up cleanup to #9605 ("check: lots of literal things"), driven by a multi-pass review (correctness, duplication, and a local-optimum/architecture audit). Sixteen well-scoped commits, each behavior-preserving except the first (a latent bug fix). No snapshots were harmed in the making of this PR
Correctness
unaryReceiverNeedsParensonly guardede_num_from_numeral, so a unary op over a negativee_typed_num_from_numeralre-emitted as--5.U8(re-tokenizes as binary minus). Merged the two from-numeral arms; added a round-trip unit test.Duplication squashes (Check.zig)
collectConstraintSignatureReachable— the signature-footprint walk was copy-pasted inboundaryDefaultLeaksIntoSignatureandrunLiteralDefaultingRounds; these must agree or a literal could be partitioned non-interfering yet warned.tryCommitNumeralCandidateandcommitLiteralGroupDefaultnow call sharedrangeNumeralDigitsFit/candidateSatisfiesRangeConstraintsinstead of hand-synced copies.commitGatheredGroup, symmetric tocommitGatheredLiteral, collapses the inlined before/commit/after warning dance.Consolidation
CheckedStaticDispatchConstraintflat fields via accessors — drop the redundantbinop_negated/num_literal(re-derived fromoriginat 5 sites) in favor ofbinopNegated()/numeralInfo(), finishing the consolidation the PR began on the type-store side.Architecture audit findings
Binop.Op → Token.Tagand looks up the parser's one table, and acomptimeassertion enforces that everyBinop.Opresolves to a token with a defined binding power (so the lookup's.?can never panic — a bad future mapping becomes a compile error).numeral > quote > interpolationwas replicated at three sites synced only by a doc comment; now oneStaticDispatchConstraint.dominantLiteralKindkernel..eqledges in the generalization-boundary scan —defaultLiteralsAtGeneralizationBoundaryre-scanned the whole module constraint list each fixpoint round; now collects the.eqledges into a front-loaded reusable buffer once per call.Tidy-ups
pub vars ontoCheckfields, gated every increment behindcomptime std.debug.runtime_safetyso release builds compile them out to a true no-op.SurfaceOriginencode/decode comptime guard + round-trip test;TestEnv.initReportBuilderhelper (collapsing 6 sites, hoisting one out of a loop); stale doc-comment fixes.Checkfor predictable memory useVerification
Every commit was implemented and independently reviewed (spec + code-quality) per task, then a final holistic review of the whole stack. Each behavior-preserving change asserts zero
test/snapshotsdrift after regeneration. Full stack: 2608 Zig unit tests pass, multi-backend eval passes, zero snapshot drift. (The binding-powercomptimeguard is compile-time-checked and adds no runtime behavior.)🤖 Generated with Claude Code