Make inclusion error output independent of the source path - #8691
Conversation
Includemod.report_error prints a non-final part of a module inclusion error as "..." when is_big holds. is_big marshalled the error part and compared the byte size with Clflags.error_size (500). The part holds locations, so the size grew with the source file name: compiling the super_errors fixture module_sig_value_arity_mismatch_nested.res from /tmp/s printed "Modules do not match:" in full, and from a 227-character directory printed "...". Fifteen super_errors fixtures changed output the same way when compiled from a long path. is_big now counts the words of the distinct blocks reachable from the error part, with string contents excluded, and Clflags.error_size is 400 words. Across the super_errors fixtures the largest part printed in full measures 298 words and the one elided part (RecordInclusion.res) measures over 469, so every snapshot keeps its output. The super_errors runner also compiles every fixture from a copy under a temporary directory more than 200 characters long and compares it with the same snapshot. Before this change that run fails for 15 fixtures (and the in-place run fails for module_sig_value_arity_mismatch_nested in an 83-character worktree); after it, all pass, both from that path and from a short /tmp path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
![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: e5512e7523
ℹ️ 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".
| (* An error part is big when its heap representation exceeds | ||
| [!Clflags.error_size] words. Each distinct reachable block counts its header | ||
| and fields; a block without scannable fields (string, float, custom) or a | ||
| closure counts its header only. String contents are excluded so that the | ||
| decision is independent of source file names stored in locations. The walk | ||
| stops as soon as the limit is exceeded, which bounds the visited list. *) |
There was a problem hiding this comment.
This changes compiler diagnostics that users see, but the commit does not update CHANGELOG.md; add an entry under the current Unreleased bug-fix section and include the PR link as required.
AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in 5eb5ae9, under the Unreleased section with the PR link.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8691 +/- ##
==========================================
+ Coverage 78.67% 79.19% +0.51%
==========================================
Files 476 473 -3
Lines 64402 64020 -382
==========================================
+ Hits 50670 50699 +29
+ Misses 13732 13321 -411
🚀 New features to boost your workflow:
|
The copies sat under a fixed 200-character padding below the temporary
directory, so on Windows the fixture paths exceeded 260 characters and bsc
could not open them ("No such file or directory" for every fixture). The
padding is now sized so the fixtures directory is 180 characters long on
every platform: still longer than any checkout path (the elision showed
from about 120), and the longest fixture path stays well below the limit.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8691 |
2 participants
Part of #8712.
Summary
Note for review:
Clflags.error_sizechanges unit from bytes of marshalled data to heap words (500 → 400); every existing snapshot is unchanged.Test plan
dune builddune build @fmtdune build --profile browser @checkmakemake liblib/outputsmake testmake test-analysismake test-toolsmake test-gentypemake test-syntaxmake checkformatnpm run check🤖 Generated with Claude Code