Skip to content

fix interactions with global names, function scopes & repl#469

Merged
davidhewitt merged 7 commits into
mainfrom
dh/load-global-by-name
May 29, 2026
Merged

fix interactions with global names, function scopes & repl#469
davidhewitt merged 7 commits into
mainfrom
dh/load-global-by-name

Conversation

@davidhewitt
Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt commented May 27, 2026

Fixes #183

I will probably extend this to also cover #423


Summary by cubic

Reworks global name resolution to be slot-safe and Python-correct without adding opcodes. Prepare eagerly allocates module slots for all global references; the VM falls back to builtins when a slot is Undefined, and NameLookup always caches into the correct slot. Fixes OOB panics, module-scope NameError semantics, and late binding in functions and the REPL.

  • Refactors

    • Module-scope names are always Global; prepare allocates/reuses a real module slot for every global reference (including unresolved).
    • Function scope never does parse-time builtin substitution; module scope only substitutes when the name isn’t already bound (including from prior REPL snippets).
    • Compiler stops emitting LoadLocalCallable; callable loads use LoadGlobalCallable. Legacy opcodes remain for decode compatibility.
    • VM adds builtin fallback for LoadGlobal and LoadGlobalCallable; delete now raises NameError when the slot is Undefined.
    • NameLookup always carries a real slot; resume caches the value into that slot (globals or frame locals).
  • Bug Fixes

    • Fixed OOB access from global X inside functions when X had no prior module slot.
    • Module-level reads now consistently raise NameError (never UnboundLocalError) and respect non-executed assignments.
    • Late binding works across functions and REPL: later def shadows builtins for both callable and non-callable loads.
    • Name lookups cache into their own slots and persist across calls.

Written for commit 607fe80. Summary will update on new commits.

Review in cubic

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 27, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks
⏩ 15 skipped benchmarks1


Comparing dh/load-global-by-name (824de9d) with main (0039ec3)

Open in CodSpeed

Footnotes

  1. 15 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

Codecov Results 📊

❌ Patch coverage is 68.98%. Project has 24136 uncovered lines.
❌ Project coverage is 66.16%. Comparing base (base) to head (head).

Files with missing lines (5)
File Patch % Lines
crates/monty/src/prepare.rs 63.57% ⚠️ 51 Missing and 10 partials
crates/monty/src/bytecode/vm/mod.rs 86.21% ⚠️ 4 Missing and 6 partials
crates/monty/src/bytecode/compiler.rs 66.67% ⚠️ 1 Missing
crates/monty/src/repl.rs 85.71% ⚠️ 1 Missing
crates/monty/src/run_progress.rs 85.71% ⚠️ 1 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    66.19%    66.16%    -0.03%
==========================================
  Files          279       279         —
  Lines        71052     71319      +267
  Branches    152853    153176      +323
==========================================
+ Hits         47030     47183      +153
- Misses       24022     24136      +114
- Partials      3993      4028       +35

Generated by Codecov Action

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/monty/src/intern.rs">

<violation number="1" location="crates/monty/src/intern.rs:913">
P1: New serialized field `global_slots` is missing `#[serde(default)]`, which breaks deserialization of previously dumped runners/snapshots.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/monty/src/intern.rs Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/monty/src/bytecode/op.rs
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/monty/test_cases/name_error__unbound_local_module.py Outdated
Comment thread crates/monty/src/bytecode/op.rs
@davidhewitt davidhewitt merged commit fbdb2e1 into main May 29, 2026
30 checks passed
@davidhewitt davidhewitt deleted the dh/load-global-by-name branch May 29, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forward function references at module scope cause NameError

1 participant