Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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.
Pull request overview
This PR fixes Nim issue #22791 by improving the compiler’s immutability detection for guard reasoning, ensuring const symbols are treated as immutable (like let) so nested case object field accessibility can be proven without triggering ProveField warnings.
Changes:
- Extend
compiler/guards.nim:isLetto treatskConstsymbols as immutable for guard analysis. - Add a regression test covering nested
caseobject access through aconstinstance underProveField+strictCaseObjects.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| compiler/guards.nim | Treats skConst as immutable in isLet, enabling guard proofs to apply to const bindings. |
| tests/objvariant/tcorrectcheckedfield.nim | Adds a regression case reproducing #22791 with nested case objects and const instance access. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
fixes #22791 This pull request introduces a minor improvement to the handling of immutable variables in the compiler and adds a new test case for nested case objects. The most important changes are: ### Compiler improvements * Updated the `isLet` guard in `compiler/guards.nim` to recognize `skConst` symbols as immutable variables, ensuring that constants are correctly identified alongside lets and other immutable types. ### Test coverage * Added a new test in `tests/objvariant/tcorrectcheckedfield.nim` for bug #22791, verifying correct pattern matching and field access in nested `case` objects with constants. (cherry picked from commit 3e2cea2)
fixes #22791
This pull request introduces a minor improvement to the handling of immutable variables in the compiler and adds a new test case for nested case objects. The most important changes are:
Compiler improvements
isLetguard incompiler/guards.nimto recognizeskConstsymbols as immutable variables, ensuring that constants are correctly identified alongside lets and other immutable types.Test coverage
tests/objvariant/tcorrectcheckedfield.nimfor bugProveFieldwarning with nestedcaseobject #22791, verifying correct pattern matching and field access in nestedcaseobjects with constants.