fix: fixed augmented assignment 🦾 - #192
Conversation
![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: f7000d86d6
ℹ️ 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: 376f4a1baf
ℹ️ 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".
Follow-up: move temporary-slot allocation metadata into the analyserThe current Longer term, the analyser should record the source-local count for the top-level frame and for each function. The compiler can then initialize
The REPL/resume path will need the top-level high-water mark to remain monotonic across analysis batches, while nested functions need their own independent count. This is not blocking for this PR. The current scan is functionally correct, but this would be a worthwhile follow-up refactor. |
376f4a1 to
6e296ed
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e296ed10d
ℹ️ 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".
timfennis
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57789c58a0
ℹ️ 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".
| .scope_tree | ||
| .resolve_call(operation, &arg_types, CallKind::Operator); | ||
| let has_op_binding = !matches!(op_binding, Binding::None); | ||
| let assign_is_eligible = Self::augmented_rhs_is_compatible(&left_type, &right_type); |
There was a problem hiding this comment.
Do not use a blanket rhs <: lhs test for every specialized assignment operator. Map -= only removes keys from the left map, so a valid expression such as let values = %{1: 10}; values -= %{1: "marker"}; cannot change the left map's Map<Int, Int> type, yet Map<Int, String> fails this compatibility check and the branch below emits a mismatched-types error. Map &= has the same issue because it only retains existing left-side entries; eligibility needs to account for the selected operator's actual mutation contract rather than requiring the entire RHS map type to be a subtype.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c2d6efba9
ℹ️ 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".
| if let Expression::Identifier { | ||
| resolved: Binding::Resolved(Candidate::Scalar(target)), | ||
| .. | ||
| } = &value.expression |
There was a problem hiding this comment.
When an indexed write targets another index expression, this direct-identifier check prevents widening the stable root binding. For example, let values = [[1]]; values[0][0] = "two"; now emits a mismatched-types error even though values is inferred and the assignment can validly widen it to List<List<Any>>; the same regression affects nested augmented assignments whose result changes type. Trace nested index targets back to their resolved root identifier and widen each affected container layer instead of treating every non-identifier container as non-widenable.
Useful? React with 👍 / 👎.
1 participant
The current augmented assignment implementation is messy and incomplete. While implementing augmented assignment for struct fields multiple problems were discovered. This PR attempts to fix these issues by having the analyzer plan the augmented assignment during analysis and generalizing the compilation of augmented assignment to also be applicable to situations like the one below: