Fix continue in for loops, nested qualified generic arguments, and two associated type lookups - #1600
Open
micahscopes wants to merge 5 commits into
Open
micahscopes wants to merge 5 commits into
micahscopes wants to merge 5 commits into
Conversation
`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.
Inside a trait, `Self::Out` was resolved against the trait instantiated with only `Self`, dropping the trait's own parameters. For a trait such as `trait T<A>`, the resulting projection had the wrong arity, so it never matched an impl that stays generic over `A`, such as `impl<A, P: T<A>> T<A> for Wrap<P>`. The method check then compared the impl's return type against the unnormalized `Wrap<P>::Out` and rejected a correct method, and callers saw the same unnormalized type. Build the trait instance from all of the trait's parameters, with `Self` in the first position, as other trait instances are built. 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 now 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.
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".
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.
This branch has not been deployed
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.
Four small bug fixes, one commit each. Each commit adds a test that fails without the fix. The first two commits stand alone; the fourth builds on the third.
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.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. The parser now lets its existing generic-argument trial parse decide, and a real shift such asvalue << 2still parses as a shift. Tests: syntax tree fixtures for the type form and for shifts after a path, a parser fixture for the expression form, and a trait resolution fixture that uses the nested argument in a trait method signature. The tree-sitter grammar still rejects the expression form, so that fixture sits incrates/uitest/fixtures/parser, outside the filestree_sitter_parse_strictchecks.Impls that stay generic over the trait's parameter now type-check. Inside
trait T<A>,Self::Outwas resolved againstTwith onlySelffilled in, which droppedA. Soimpl<A, P: T<A>> T<A> for Wrap<P>withfn f(_ a: A) -> Self::Outwas rejected with "expectedWrap<P>::Out". The trait instance is now built from all of the trait's parameters. Tests: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. That error now 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. Bounds on the trait's own associated types, such astype Inner: Arrow, are not used. When two supertraits both declare the name, the existing ambiguity error is reported, with a hint to write<Self as Left>::Dom. Tests: type check fixtures for the single-supertrait case, the ambiguous case, and the associated type bound case.These changes merge without conflicts with the open PRs #1576, #1582 and #1589.