Repository navigation
Fix continue in for loops, nested qualified generic arguments, and associated type lookup through bounds - #1600
Merged
Merged
Conversation
micahscopes
marked this pull request as ready for review
September 25, 2026 08:10
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821230f637
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
`continue` inside a `for` loop jumped straight back to the loop condition, skipping the index increment that only ran when the body fell through. The loop therefore revisited the same element forever, or until something else in the body broke out of it. Lower the increment into its own block and make it the loop's continue target, so normal fallthrough and `continue` both advance the sequence before the condition is checked again. `while` loops keep the condition as their continue target. The fe test fixture covers an unconditional `continue`, `continue` in a nested `for`, `continue` followed by `break`, and a `while` nested in a `for`. The Sonatina IR snapshots for existing `for` loops change only by the new increment block and its jump.
A path segment skipped generic argument parsing whenever the next tokens were `<<`, treating them as a left shift. In `Wrapped<<T as Model>::Point>` that left the arguments off the `Wrapped` segment: in type position they attached to the surrounding path type, where lowering does not read them, and in expression position the parser reported a malformed shift. Drop the `<<` exclusion and let the existing generic argument dry run decide. A real shift such as `value << 2` does not parse as a generic argument list, so it still parses as a shift. Tests: a syntax tree fixture for the type form, a parser fixture for the expression form (a call and a struct literal), a type check fixture that uses the nested argument in a trait method signature, and a syntax tree fixture that keeps shifts after a path parsing as shifts.
Mainline #1535 builds `Self::Out` inside a generic trait from all of the trait's parameters, so `impl<A, P: T<A>> T<A> for Wrap<P>` with `fn f(_ a: A) -> Self::Out` now type-checks. Its fixture covers bare associated names on concrete impls; these fixtures cover an impl that stays generic over the trait's parameter. The fe test fixture runs such an impl end to end. The type check fixture keeps the diagnostic for a genuinely wrong return type, which names the impl's actual `(P::Out, A)` instead of `Wrap<P>::Out`.
Inside a trait, `Self::Name` was looked up only among the trait's own associated types. In `trait Eval: Arrow`, `Self::Dom` failed with "`Dom` is not found" even though every `Self` of `Eval` also implements `Arrow`, which declares `Dom`. When the trait itself has no associated type of that name, look through the bounds implied for `Self` by the trait's declared supertraits (transitively) and return every one that declares it. Bounds on the trait's own associated types, such as `type Inner: Arrow`, have `Self::Inner` as their self type and are skipped, so `Self::Dom` does not resolve through them. One match resolves the name; two or more go through the existing ambiguity diagnostic, which suggests the qualified form such as `<Self as Left>::Dom`. Type check fixtures cover the single-supertrait case, which now has no diagnostics, the ambiguous two-supertrait case, and an associated type bound that must not be used.
Review of the nested qualified generic argument fix asked whether `value << <T as Model>::BITS > limit` could now be claimed by the generic argument trial on `value`, leaving `limit` disconnected. It is not: that expression has three `<` tokens (`<<`, then the `<` of `<T`), so the trial would need `< <T as Model>::BITS >` to parse as a type argument, which it does not, and the parser falls back to the shift. The result is `(value << <T as Model>::BITS) > limit`, as before the fix. This fixture pins that behavior with executed checks: `>`, `>=`, the unspaced `value<<<T as Model>::BITS`, a following `>>`, and the comparison as a call argument. The unspaced form is deliberate, so the formatter's spacing change is expected; the formatter round-trip test compares meaning.
micahscopes
force-pushed
the
pr/upstream-small-fixes
branch
from
September 28, 2026 00:45
9bab302 to
e15f10a
Compare
micahscopes
changed the title
continue in for loops, nested qualified generic arguments, and two associated type lookupscontinue in for loops, nested qualified generic arguments, and supertrait associated type lookup
This was referenced Sep 28, 2026
The advance block that `continue` now targets was emitted even when every
path through the loop body leaves the loop through `break`, `return`, or a
diverging call. The orphaned block read the loop index, which semantic
normalization cannot resolve in an unreachable block, so valid functions
such as `for i in 0..3 { break }` failed with an internal borrow checking
error.
Record whether a `continue` emits an edge from a live block. When neither
fallthrough nor `continue` reaches the advance block, close it with a
self-loop, as dead `if` and `match` joins already are.
Removing the `<<` exclusion let any successful generic-argument trial claim a left shift. A cast inside a shift is also a valid qualified type, so `value << bits as u256 >> 1` parsed as `value< <bits as u256> >` followed by a stray `1`. Path segments after the first are parsed as types, so `M::BITS << bits as u8 >> 1` broke the same way. After a path segment, `<<` now opens generic arguments only when a qualified path follows, as in `Wrapped<<T as Model>::Point>`. No shift operand continues with `>::`, so that prefix decides it and the arguments are parsed directly instead of trying the whole list first. This also drops the repeated full-list trial at each nesting level of `Wrap<<... as Model>::Point>`. `QualifiedTypeScope` now reports a missing `as` instead of asserting, so it can recognize the prefix. A bare qualified first argument, `Wrapped<<T as Model>>`, is again not read as generic arguments; `Wrapped< <T as Model> >` still is. Tests: the regressed shifts added to the `path_lshift` syntax tree fixture, a recovery fixture for a qualified argument list missing its closing `>`, and an executed fixture for each regressed shift.
The formatter printed `Wrapped< <T as Model>>` as `Wrapped<<T as Model>>`. The parser reads `<<` as generic arguments only before a qualified path, so the formatted code became a shift in expressions and detached the argument from `Wrapped` in types. Format such lists with inner spaces, `Wrapped< <T as Model> >`, and leave qualified paths such as `Wrapped<<T as Model>::Point>` unchanged.
`Self::Name` inside a trait returned as soon as the trait's implied supertrait bounds produced a candidate, skipping the scan of the caller's assumptions that follows it. A method-level `where Self: Extra` therefore never competed with an inherited associated type: with `trait Sub: Base` and `fn take(_ x: Self::Item) where Self: Extra`, where `Base` and `Extra` both declare `Item`, `Base::Item` was selected silently. An impl written against `Extra::Item` was then rejected for the inherited type. The generic receiver form `fn take<T: Base>(_ x: T::Item) where T: Extra` reports the ambiguity correctly, because both bounds reach it through one assumption list. A trait's own `Self` bounds are absent from `assumptions` in header positions such as a method signature, where assuming `Self: Trait` while the trait's interface is still being lowered can recurse through the in-progress definition. The two candidate sources are therefore disjoint and must be merged rather than one preferred. Candidates are also deduplicated, so a method that restates a bound the enclosing trait already implies is not reported as ambiguous with itself.
Candidates were compared by normalizing each one under the caller's assumptions alone. A diamond that reaches one declaration twice, once with an equality, produced two keys and was reported as ambiguous with itself: with `trait Left: Base<Item = u256>` and `trait Right: Base`, the candidate from `Left` carries the binding and normalizes to `u256`, while the binding-free candidate from `Right` stays an unresolved projection because the equality only appears once the bounds are extended. Normalization now uses the implied-bound closure of the assumptions, plus the enclosing trait's own implied bounds when the receiver is that trait's `Self`, which header positions withhold from `assumptions`. Both the `Self` and the generic receiver forms of the diamond are affected; the latter was wrong before the inherited lookup existed. Each candidate is still keyed on its own equality first, so paths that disagree on the bound type remain ambiguous instead of collapsing to whichever was seen first.
Telling generic arguments from a shift or a comparison needs a speculative parse, and four of them run over the same syntax: the qualified-type check, the `<<` check, the const-call check, and the argument-list check. Each level of a nested argument probed the positions inside it, and every enclosing level probed them all again, so cost grew about fivefold per level of depth. `Wrap<<T as Model>::Point>` nested twelve deep is 299 bytes and took about 96 seconds to parse; nine deep took 0.8 seconds. `dry_run` restores the stream, position, buffered trivia and errors, and leaves the tree builder alone, so a probe that records no error is a function of the position it starts from and of whether newlines are trivia there. The input does not change during a parse, so those outcomes are now kept and reused. Twelve deep parses in milliseconds and two hundred deep, at 4.8 KB, in about a tenth of a second. Outcomes reached through error recovery are not reused. `recover` scans the whole enclosing scope stack for a token to stop at, so how far it consumes, and with it the outcome, can depend on where the probe ran from rather than only on the position. Those are re-run as before, which leaves malformed input exactly as it was.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccd46d3070
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1b41e2f6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Path segments decide a `<<` by whether a qualified path follows, but `is_method_call` still rejected one outright, so `receiver.take<<M as Model>::Point>(42)` parsed `.take` as a field and the `<<` as a shift. The type and static path forms the same rule already fixed made the inconsistency visible: `Wrapped<<M as Model>::Point>::new(p)` parsed while the method form did not. Both now call `lshift_opens_generic_args`, so the rule lives in one place and cannot drift. A `<<` that is not followed by a qualified path is still a shift, as in `value.bits << 1`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd37d81e6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Only a probe that reaches its outcome without error is independent of the scope stack it ran under, but the check for that compared error-list lengths, and `recover` does not add to the error list while in dry run mode: it raises the dry run's own flag instead. A probe whose recovery consumed tokens therefore looked error free and had its outcome cached, which is the context-dependent case the check exists to exclude. Both signals are now consulted. No input was found where the previous check produced a wrong tree or diagnostic, so this closes the hole rather than fixing an observed misparse.
The compiler accepts `Wrapped<<M as Model>::Point>::new(point)` but the tree-sitter grammar reported an error on it, so editors flagged valid code. In a type position the scanner already emitted the first `<` of a `<<` as a generic open, because a comparison cannot appear there and nothing else the `<<` could be remains. In an expression position both a generic open and a comparison are possible, so it deferred to the internal lexer and got a shift. The scanner now applies the rule the parser uses: it scans the second `<` to its matching `>` and takes the `<<` as a generic open only when `::` follows, since no shift operand continues with `>::`. Shifts are unaffected, including `value << bits as u256 >> 1` and a shift whose operand is qualified, as in `value << <M as Model>::BITS > limit`, whose closing `>` is a comparison. The vendored wasm is regenerated, as its input hash covers the scanner. Coverage goes in the parser crate's tree-sitter tests rather than a fixture directory, because both tree-sitter fixture suites skip `uitest/fixtures/parser`, and it pins the shift forms alongside the accepted ones.
Deciding whether a `<<` opens generic arguments means scanning ahead for the
`>` that closes a qualified path, and the scan runs on to the end of the
enclosing block before giving up. It read every character as code, so a `>::`
inside a string literal or a comment closed the angle nesting and made an
ordinary shift lex as a generic open, leaving ERROR nodes on valid code:
let a = x << y
let s = "a>::b"
`a <<= y` was affected the same way, since the shift-assign reached the
lookahead too. Only editor highlighting was wrong; the compiler decides this
on tokens and was never confused.
String literals and comments are now skipped whole, and a `=` right after the
second `<` settles `<<=` as a shift before any scanning. The vendored wasm is
regenerated, as its input hash covers the scanner.
The sibling single-`<` scan has the same hole for string literals, which is
left alone here: it is pre-existing, plain comparisons are far more common
than shifts, and the change deserves its own coverage.
A probe outcome is only reusable if it depends on nothing but the input and the position, and error recovery was the one thing that broke that: `recover` searches the whole enclosing scope stack for a token to stop at, so how far it consumes, and therefore the outcome, depends on where the probe ran from. Excluding those outcomes from the cache does not work. The check could not see recovery inside a *nested* dry run, which is where it happens once probes nest, because an inner `dry_run` discards both the error list and the dry run's own flag. Counting every error in a never-reverted counter closes that but un-caches far too much: malformed nesting at depth 10 went from 13 ms to 99 ms, growing about 2.5x per level. Exempting a recovery whose match resolved inside the probe is unsound, because another enclosing context can accept an earlier token and stop the scan sooner. So the dependence is removed rather than detected. A probe records its scope depth and `recover` searches no further out, which is what a probe already asks -- would this construct parse here, on its own -- and a probe's tree and errors are discarded either way. Every outcome is then a function of the cache key, so the cache no longer needs to judge which ones qualify. Malformed input is memoized as a result, which the cache never managed before: about 20-35 us at any depth, against 13 ms at depth 10 growing over twofold per level. An editor sees malformed input on every keystroke, so that is the case that matters most. The new test pins the reuse, not the confinement, which has no effect on timing; that nothing observable changed is what the rest of the suite shows.
When every body path leaves the loop, `advance_bb` is closed with a self-loop and nothing can reach it, but the increment was emitted into it anyway and relied on `push_stmt` and `set_terminator` no-opping on a terminated block. `emit_expr` still allocated its temps, so each such loop left two locals declared and never assigned. The emission now sits in the arm that knows the block is reachable, and the self-loop in the other. Same CFG either way.
The skipped advance block fix (53b847b, 8ad84af) builds the increment when the body falls through or a reachable `continue` targets it. `continue_and_break_preserve_sequence_order` already covers a body that never falls through while a `continue` reaches the increment: an `if` with no `else` holds the `continue`, and a `break` follows it. These cases add three shapes that one does not reach: - `continue` in one arm of an `if`/`else` and `return` in the other, so the `if` join is unreachable and the other path leaves the function instead of the loop; - `continue` and `break` in the two arms of a `match`, so the reachable `continue` comes from the `match` lowering; - an inner loop whose body always breaks inside an outer loop that advances by falling through, so the inner loop skips its increment while the outer loop keeps its own. They sit in `for_continue_advances.fe` next to the existing `continue` cases and the cases where every path exits. All three pass with the fix.
For a type parameter receiver, associated type lookup tried each bound in scope by unifying the receiver with the bound's self type. It first replaced the parameters in that self type with fresh inference variables, so a bound's bare `T` became a variable that unified with any receiver. `U::Dom` in `fn f<T: Arrow, U>(_ x: U::Dom)` therefore resolved to `T::Dom`, and so did `Self::Dom` in `trait Eval<T: Arrow>`, with or without a `where` clause. Mainline has the same behavior. On mainline the leak makes `Self::Dom` in `trait Eval<T: Arrow>: Arrow` silently mean `T::Dom`, so an impl written against the supertrait's `Dom` is rejected. With inherited and contextual candidates merged (938b152), the leaked bound now shows up as a second `Arrow` candidate, and the reference is reported as ambiguous (2-0009). The bounds in scope name the same parameters as the receiver, so match their self types as they are. Method selection already keeps bound arguments intact for type parameter receivers for the same reason. The type check fixture covers `Self::Dom` in a generic trait with an inline bound and with a `where` clause, `U::Dom` next to `T: Arrow` in a function, `T::Dom` in the same function, which still resolves, and `Self::Dom` from a supertrait next to `T: Arrow`, which resolves without ambiguity. Failing first: at 8ad84af the snapshot lacks the three "not found" errors. That run predates the supertrait case. For that case, on master the trait resolves `Self::Dom` to `T::Dom` without an error: with `type Dom = u32` for `D` and `type Dom = bool` for `S`, the valid `impl Eval<D> for S { fn eval(_ v: bool) -> u256 { 0 } }` is rejected with "expected `u32`, found `bool`". At 8ad84af with only this fixture added, the same trait reported 2-0009 with `Arrow` listed twice; that was observed in a session log, and the probe binary was not kept.
When `Self::Out` was ambiguous between `Gen<u8>` and `Gen<u16>`, both candidate labels read `Gen` and the hint suggested `<Self as Gen>::Out`, which does not pick either one. The same happened for an associated constant with impls of `Gen<u8>` and `Gen<u16>`. Render the trait in candidate labels and hints with its type arguments, as it is written after `as`, and leave out associated type bindings, which are not part of that path. Trait instances that compare equal by name and self type are now ordered by that rendering, so the hint picks the same candidate every time. Traits without type arguments print as before, so existing snapshots do not change. Type check fixtures cover the two-supertrait `Gen<u8> + Gen<u16>` case and the associated constant case.
Cover the byte layout and stride of packed records stored in a memory array, and writes through a raw pointer to a nested field and to an array element of one record. The test passes on the current compiler; it guards the projection path against regressions.
Layout recurrence checking treated any repetition of a nominal type through ordinary fields as growth, so a struct holding `Option<Argument>`, where `Argument` holds `Option<u256>`, was rejected as an expanding type although its layout is finite. For two ADT frames, keep an ancestor as a recurrence only when its type structurally embeds the current type. Provider families keep the stricter rule.
The finite nested layout fix narrows which earlier types count as a recurrence, so it needs evidence that the check still stops on types that grow. The existing test covers growth by duplicating an argument, `Growing<T>` holding `Growing<(T, T)>`. Add growth through an inserted wrapper, directly (`Wrapped<T>` holding `Wrapped<Option<T>>`) and through a second type (`Ping<T>` holding `Pong<(T, u8)>`, which holds `Ping<Option<T>>`). The test queries the layout schema directly, since a growing struct is otherwise rejected earlier as a recursive type definition, and runs the check on a worker thread under a 60 second deadline, so a walk that never ends fails the test instead of hanging it. Both types are reported as non-regular cycles well under the deadline. The new test and the existing one read the schema through one helper.
micahscopes
changed the title
continue in for loops, nested qualified generic arguments, and supertrait associated type lookupcontinue in for loops, nested qualified generic arguments, and associated type lookup through bounds
sbillig
approved these changes
Sep 29, 2026
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
2 participants
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.
Small bug fixes to the loop lowering, the parser and associated type lookup, plus tests for a fix that mainline #1535 made, a clearer ambiguity hint, and a layout fix and a packed record layout test moved here from #1589. Sean's commits fix the
forloop regression the first commit introduced, rework how the parser decides a<<, extend that decision to method calls, the formatter and the tree-sitter grammar, keep deep nesting fast, and make supertrait lookup merge and compare candidates correctly. Each fix has a test that fails without it.continuein aforloop now advances.continuejumped back to the loop condition without running the index increment, so the loop revisited the same element. For example, infor i in 0..5 { continue }the index never moved past 0. The increment now has its own block, and both the end of the body andcontinuego through it.whileloops are unchanged. Test:crates/fe/tests/fixtures/fe_test/for_continue_advances.fe(unconditional, nested, and break-after-continue cases, plus awhileinside afor). In four Sonatina IR snapshots of existingforloops, the existing increment moves into its own block, the body gains one jump to it, and blocks renumber.That change first built the increment block for every loop, so a body that always ends in
break,returnor a diverging call left an unreachable block that read the loop index, and loops such asfor v in a { return v }failed with an internal borrow checking error. Sean's 53b847b and 8ad84af fix this: lowering records whether a reachablecontinuetargets the loop, and when neither fallthrough norcontinuereaches the increment, the block is closed with a jump to itself, as unreachableifandmatchjoins are, and nothing is emitted into it. His cases infor_continue_advances.fecover a body that always breaks, always returns, exits on every branch, or diverges. Three more cases there cover shapes the existingcontinuecases do not:continuein one arm of anif/elsewithreturnin the other,continueandbreakin the arms of amatch, and an inner loop that always breaks inside an outer loop that advances.Wrapped<<T as Model>::Point>now parses as a generic argument. The parser treated the<<as a left shift and skipped the generic arguments. In type position they ended up attached to the whole type instead ofWrapped, where lowering ignores them. In expression position, for exampleWrapped<<M as Model>::Point>::new(point), the parser reported a syntax error. After a path segment, a<<now opens generic arguments only when a qualified path follows it, since no shift operand continues with>::(Sean's fd291c8). An earlier version of this PR let a generic-argument trial parse decide, which misread shifts whose operand contains a cast, such asvalue << bits as u256 >> 1. A bare qualified first argument,Wrapped<<T as Model>>, is not read as generic arguments;Wrapped< <T as Model> >is, and the formatter now keeps that spacing (a2a1699). The same rule applies to method calls such asreceiver.take<<M as Model>::Point>(42)(a1b41e2) and to the tree-sitter grammar, whose scanner skips string literals and comments while it looks for the closing>(cd37d81, 392092b; the vendored wasm is regenerated). The speculative parses behind these decisions are now memoized per position, with error recovery confined to the scopes a probe opened, so twelve levels ofWrap<<T as Model>::Point>parse in milliseconds instead of about 96 seconds (ccd46d3, 14f04dd, 0acbfa0). Tests: syntax tree fixtures for the type form and for shifts after a path, including the cast forms; an error recovery fixture for a missing closing>; a parser fixture for the expression form; executed fixtures for the regressed shifts, for a method call with a qualified argument, and for a shift compared against a qualified associated constant; formatter snapshots; tree-sitter tests for the accepted and the shift forms; a nesting-depth test for the parser; and a trait resolution fixture that uses the nested argument in a trait method signature.Tests for impls that stay generic over the trait's parameter. Inside
trait T<A>,Self::Outused to dropA, soimpl<A, P: T<A>> T<A> for Wrap<P>withfn f(_ a: A) -> Self::Outwas rejected with "expectedWrap<P>::Out". Mainline #1535 fixed this with the same change this PR made, building the trait instance from all of the trait's parameters, so this PR now only adds tests for that case:crates/fe/tests/fixtures/fe_test/generic_trait_arg_impl_assoc_type.feruns such an impl, and a type check fixture keeps the error for a genuinely wrong return type, which names the real type,(P::Out, A).Self::Domfrom a supertrait now resolves. Intrait Eval: Arrow { fn eval(_ value: Self::Dom) }, whereArrowdeclarestype Dom,Self::Domfailed with "Domis not found". If the trait has no associated type with that name, lookup now checks the bounds its supertraits imply forSelf, starting from the same trait instance #1535 builds forSelf::Out. Bounds on the trait's own associated types, such astype Inner: Arrow, are not used. Sean's 938b152 merges these inherited candidates with the bounds in scope at the reference instead of returning early, so a method's ownwhere Self: Extracompetes with an inheritedItemand is reported as ambiguous rather than silently losing, and a bound reached twice is not ambiguous with itself. His 6b69446 compares candidates under the implied bounds, so a diamond that reaches one declaration throughLeft: Base<Item = u256>andRight: Baseresolves, while paths that bind different types stay ambiguous. When two supertraits both declare the name, the ambiguity error has a hint to write<Self as Left>::Dom. When the two are the same generic trait, as inGen<u8> + Gen<u16>, the candidates and the hint now include the type arguments,<Self as Gen<u16>>::Out; associated constants get the same change. Tests: type check fixtures for the single-supertrait case, the ambiguous case, the generic ambiguous case for types and constants, the associated type bound case, a method-levelwherebound, a repeated bound, and the diamond with an equality, forSelfand for a generic receiver, with and without conflicting equalities.U::Domno longer resolves through a bound on another parameter. Infn f<T: Arrow, U>(_ x: U::Dom),U::Domsilently meantT::Dom, and so didSelf::Domintrait Eval<T: Arrow>. The lookup replaced the parameters in each bound's self type with fresh inference variables before matching, so a bound onTmatched any receiver. On mainline the leak makesSelf::Domintrait Eval<T: Arrow>: Arrowsilently meanT::Dom, so an impl written against the supertrait'sDomis rejected. With inherited and contextual candidates merged (938b152), the leaked bound now shows up as a secondArrowcandidate, and the reference is reported as ambiguous. The bounds in scope name the same parameters as the receiver, so they are now matched as written, as method selection already does. That removes the leaked candidate, and the reference resolves to the supertrait'sDom. Test: a type check fixture for the trait case with an inline bound and with awhereclause, the function case,T::Domin the same function, which still resolves, and the supertrait case, which resolves without ambiguity.Finite nested layouts are no longer rejected as expanding.
struct Call { argument: Option<Argument> }withstruct Argument { label: Option<u256> }was rejected as an expanding type, because the layout recurrence check treated any repeat of a nominal type (Optionhere) through ordinary fields as growth. For two struct or enum frames, an earlier type now counts as a recurrence only when it structurally embeds the current one: the same constructor with each argument embedded pairwise, or the earlier type embedded in one of the current type's arguments. Const arguments count as one class, and associated types, type variables and invalid types always count, since the check cannot see into them. Provider families keep the old, stricter rule. The check still terminates on growing types, because the types met along one path are trees over finitely many constructors, and by Kruskal's tree theorem an infinite path would meet an earlier type embedded in a later one; the theorem gives no useful depth bound, so a test also checks the time directly. Tests:finite_nested_options.fenow has a finite layout,expanding_structural_arguments.fe(Growing<T>holdingGrowing<(T, T)>) is still reported as a non-regular cycle, andexpanding_through_wrappers.fecovers growth through an inserted wrapper, directly (Wrapped<T>holdingWrapped<Option<T>>) and through a second type (Ping<T>toPong<(T, u8)>toPing<Option<(T, u8)>>), with the check run under a 60 second deadline so a walk that never ends fails instead of hanging. These tests query the layout schema directly, because a growing struct is otherwise rejected earlier as a recursive type definition. This change and its tests moved here from #1589.A test for packed record layout moved here from #1589.
crates/fe/tests/fixtures/fe_test/raw_record_layout.fechecks the byte layout and stride of packed records stored in a memory array, and writes through a raw pointer to a nested field and to an array element of one record. It passes on the current compiler and guards the projection path against regressions. The #1589 review asked for it to move out of that PR.Testing. The full workspace test run passed locally, 3611 of 3611 tests, before a last test-only change that folded one fe_test fixture file into another and shared a helper between two layout tests. The affected tests (the
for_continue_advances.ferun and format round trip, and all oflayout_evidence) pass at the head of this branch, which has 3609 tests in total, and fmt, clippy and the newsfragment check are clean.Merging with the open PRs: #1582 and #1606 merge cleanly. #1576 conflicts in the vendored tree-sitter wasm and its input hash,
crates/fmt/tests/format_snapshots.rsandcrates/parser/src/parser/param.rs, all from the parser and formatter commits. #1589 as published conflicts incrates/hir/src/analysis/ty/layout_holes.rsandcrates/hir/tests/layout_evidence.rs, because it still carries the layout fix that moved here; the conflicts go away once #1589 drops it.