exclude noreturn, undefined and @compileError peers in branchin… - #3242
Open
DaliVana wants to merge 1 commit into
Open
exclude noreturn, undefined and @compileError peers in branchin…#3242DaliVana wants to merge 1 commit into
noreturn, undefined and @compileError peers in branchin…#3242DaliVana wants to merge 1 commit into
Conversation
…g type resolution Fixes zigtools#3210
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.
Fixes #3210
switchandiftype resolution now ignores branches that never produce a value, mirroring Zig's peer type resolution:unreachable/break/continue/return(noreturnvalues),undefined, and@compileError(...)no longer prevent the remaining peer from determining the result. All four examples from the issue now resolve toS.Root cause
Two defects, both around
Type.fromEither:resolvePeerTypesopens withif (a.is_type_val or b.is_type_val) return null;, so the existing exclusions inresolvePeerTypesInternal(.compile_error => return b,.noreturn => return b) never run when one peer is a type likeS. (This is whyx orelse unreachablealready worked — no type-valued peer there.)fromEitherreturnsnullon anyis_type_valmismatch, so anoreturn/undefinedvalue entry next to the typeSpoisoned the whole result (issue examples B and C resolved to nothing)..compile_errorentries were skipped by the mismatch check but still entered the deduplicator, producing the messyeither { S, @compileError(...) }from example D.Fix
fromEitherfilters never-producing entries up front:.compile_errorentries, and.ip_indexentries whose type is.noreturn_typeor.undefined_type. Keying onpayload.type(notpayload.index) keeps the literal type valuesnoreturn/undefinedas valid peers (if (cond) u8 else noreturnstill resolves to an either type). The now-dead.compile_errorspecial-casing in the either-construction is removed.Design notes:
entries.len == 1early return, so single-branch resolution (ifwithoutelse) is unchanged.undefinedbranch determines the result; otherwise anoreturn-typed entry is preferred over.compile_errorso the result does not depend on branch order ({ unreachable, @compileError("") }resolves tonoreturnin both arm orders; all@compileErrorstill resolves to the.compile_errortype for richer hover output).resolvePeerTypes'is_type_valguard is intentionally left untouched to keep the change scoped to branching constructs; hoisting the noreturn/compile_error exclusions above that guard could be a follow-up.Tests
tests/analysis/switch.zig: the issue's repro shapes (unreachable/undefined/@compileErrorarms each resolving to(type)(A)), plus all-noreturn switches in both arm orders and anunreachable+undefinedswitch pinning the fallback ordering.tests/analysis/either.zig: the four existingif-based assertions encoded the buggy behavior ((unknown)()/(either type)()) and now expect(i32)(); a new case pins that the literal type valuenoreturnis not filtered.tests/analysis/peer_type_resolution.zig:undefined_0/undefined_1value-level pair alongside the existingnoreturn_0/noreturn_1, since thefromEitherfilter is the only mechanism that excludesundefinednext to a non-ip_indexpeer (resolvePeerTypesInternalhas no.undefinedarm).