feat[lang]!: make immutable accessed through self#5035
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: e7d97cdb9a
ℹ️ 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".
| self_assigns = constructor.ast_def.get_descendants( | ||
| vy_ast.Assign, {"target.value.id": "self"} | ||
| ) |
There was a problem hiding this comment.
When a constructor assigns a numeric immutable and then updates it with self.VALUE += 1, this validator only counts Assign nodes, while FunctionAnalyzer.visit_AugAssign still permits CODE-location writes inside constructors. That lets an immutable be modified after its required assignment, bypassing the single-assignment rule that this commit is trying to preserve for self.VALUE = ... duplicates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not sure the previous system caught that either, but will fix
There was a problem hiding this comment.
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5035 +/- ##
==========================================
- Coverage 92.92% 92.86% -0.07%
==========================================
Files 188 188
Lines 27820 27832 +12
Branches 4828 4831 +3
==========================================
- Hits 25853 25846 -7
- Misses 1316 1331 +15
- Partials 651 655 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| raise ImmutableViolation( | ||
| "Immutable value cannot be modified after assignment" | ||
| ) | ||
| info.var_info._modification_count += 1 |
There was a problem hiding this comment.
uhhhhhh why was this entire (correct) section removed?
There was a problem hiding this comment.
why are the tests passing while this correct section was removed? we need to add a test for this case!
There was a problem hiding this comment.
btw, for this PR you can ignore the failing gas-bench CI run as this is a breaking change which is currently incompatible with the latest snekmate version.
3 participants
What I did
closes: #2695
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture