import: project-first resolution + stdlib-collision warning (#821) - #856
Merged
Conversation
import name now tries name.eigs (script-relative, plus the chain's other locations and the eigs_modules walk) BEFORE the stdlib's lib/name.eigs. Under stdlib-first, a project file sharing a stdlib module's name was silently shadowed — every member access on the intended module read null, and the first symptom was an unrelated-looking downstream type error (dynamics' physics.eigs, F-DYN-8). The stdlib namespace grows release to release, so any consumer was one new stdlib module away from silent capture. Both request shapes are always probed: a name matching both distinct files prints a one-line stderr warning (once per name per process) naming the file used and the file shadowed. Sweep of the repo and all 15 consumer repos found zero imports whose resolution flips (dynamics never imports physics — it uses the load_file workaround, which keeps working). Closes #821 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements project-first module import resolution and adds a deduplicated collision warning when a module name matches both a project file and a stdlib module (#821), with accompanying tests and documentation updates.
Changes:
- Switch import resolution order to try
name.eigsbeforelib/name.eigs, and emit a stderr warning on collisions (once per name per process). - Add regression tests for shadowing behavior and for the collision-warning dedup behavior.
- Update SPEC and CHANGELOG to document the new resolution order and warning behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_import.eigs | Adds regression coverage ensuring project modules shadow stdlib modules (#821). |
| tests/run_all_tests.sh | Asserts the new collision warning behavior on stderr (warn-once + no false positives). |
| src/vm.c | Implements project-first import resolution and collision warning with process-lifetime dedup. |
| docs/SPEC.md | Documents the updated import resolution order and collision warning semantics. |
| CHANGELOG.md | Notes the behavior change and rationale in Unreleased changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+778
to
+796
| #if !EIGENSCRIPT_FREESTANDING | ||
| /* #821: import-collision diagnostic dedup — one warning per module name | ||
| * per process (an import statement re-resolves on every execution, and a | ||
| * collided name imported from several files would otherwise repeat the | ||
| * same line). Process-lifetime by design: still-reachable at exit, which | ||
| * LeakSanitizer does not report. Main-thread only, like the module cache. */ | ||
| static int import_collision_first_report(const char *name) { | ||
| static char **warned = NULL; | ||
| static size_t warned_n = 0, warned_cap = 0; | ||
| for (size_t i = 0; i < warned_n; i++) | ||
| if (strcmp(warned[i], name) == 0) return 0; | ||
| if (warned_n == warned_cap) { | ||
| warned_cap = warned_cap ? warned_cap * 2 : 8; | ||
| warned = xrealloc_array(warned, warned_cap, sizeof(char *)); | ||
| } | ||
| warned[warned_n++] = xstrdup(name); | ||
| return 1; | ||
| } | ||
| #endif |
Comment on lines
+5274
to
+5275
| if (user_hit && stdlib_hit && | ||
| import_collision_first_report(name)) { |
| # asserted here: exactly ONE line for a collided name however many import | ||
| # statements execute (warn-once dedup), and NO line when only the stdlib | ||
| # matches. Runs in a temp dir so the probe cannot touch tree state. | ||
| SH821_DIR=$(mktemp -d) |
Comment on lines
+5291
to
+5292
| if (!user_hit) | ||
| memcpy(path_buf, stdlib_buf, sizeof(path_buf)); |
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
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.
Closes #821
What
import nameresolution order flips to project-first:name.eigs(script-relative, plus the chain's other locations and theeigs_moduleswalk) is tried before the stdlib'slib/name.eigs. A name matching both distinct files now prints a one-line stderr warning (once per name per process) naming the file used and the file shadowed.Why
Under stdlib-first, a project file sharing a stdlib module's name was silently shadowed — every member access on the intended module read
null, with the first symptom an unrelated-looking downstream error (dynamics'physics.eigs, F-DYN-8). The stdlib namespace grows every release, so any consumer was one new stdlib module away from silent capture of an existing import. Silent-tolerance class: two plausible resolutions, one picked with no signal — the warning is the load-bearing half whichever order wins.Blast radius
Swept the repo and all 15 consumer repos for files sharing a stdlib module name: 5 collisions exist (
dynamics/physics.eigs,tidelog/src/store.eigs,Tidepool/src/audio.eigs,iLambdaAi/lib/validate.eigs,ouroboros/test/programs/observer.eigs) but zero are imported — no resolution flips anywhere. dynamics uses theload_fileworkaround, which keeps working.Tests
tests/test_import.eigs: project file with a stdlib name wins (fails on stdlib-first — the dict has noSHADOW_MARKERkey).tests/run_all_tests.sh[36]: shell-level stderr gate — exactly one warning line across two import statements of a collided name (warn-once dedup), zero on a clean stdlib import.Gates
detect_leaks=1: 3737/3737 passed, leak tally 0 (warn-once list is still-reachable by design, not reported).Closes #821
🤖 Generated with Claude Code