Declare exports left unbound by a toplevel throw - #8692
Conversation
When a toplevel binding always throws, its `let` compiles to the `throw`
statement alone, and Js_output.concat drops every later group. The export
list still names those identifiers, so the module is invalid JavaScript:
`node --check` rejected tests/tests/src/conditional/cond_a.mjs ("Export 'u'
is not defined in module") and cond_a_none.mjs ("Export 'A' is not
defined in module").
Lam_compile_main now appends a `let x;` declaration, after the throw, for
each exported identifier the finished module body does not declare. Only
modules whose body always throws are affected; make test regenerates no
other output.
cond_a_load_test dynamically imports both modules and expects loading to
reject with Assert_failure. Before the fix both cases fail with the
SyntaxError; after it both pass.
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: 4584ee8b9f
ℹ️ 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".
| if output.output_finished = True then | ||
| declare_undeclared_exports meta.exports block |
There was a problem hiding this comment.
This changes compiler output for affected user programs from invalid to valid JavaScript, making it a user-facing bug fix, but the commit does not add the required entry under the current Unreleased bug-fix section of CHANGELOG.md; without it, the fix will be omitted from the release notes.
AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in 59571ed, under the Unreleased section with the PR link.
| alone. The identifiers those groups bind are then undeclared, and an export | ||
| list naming them is a JavaScript syntax error. This appends a declaration | ||
| for each such exported identifier after the [throw]. *) | ||
| let declare_undeclared_exports (exports : Ident.t list) (block : J.block) : |
There was a problem hiding this comment.
The message for commit c00d4f6c38e4239dcb5d2ed117c2b7b0b7e598cd has no Signed-Off-By trailer, so the contribution does not meet the repository's required DCO commit standard and cannot be accepted as submitted.
AGENTS.md reference: AGENTS.md:L68-L70
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Every commit on this branch carries a Signed-off-by trailer (git log --format='%(trailers:key=Signed-off-by)' origin/master..HEAD lists one per commit), so there is nothing to add.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8692 +/- ##
=======================================
Coverage 79.19% 79.19%
=======================================
Files 473 473
Lines 64020 64031 +11
=======================================
+ Hits 50699 50712 +13
+ Misses 13321 13319 -2
🚀 New features to boost your workflow:
|
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-8692 |
2 participants
Part of #8712.
Summary
letcompiles to thethrowstatement alone, and Js_output.concat drops every later group. The export list still names those identifiers, so the module is invalid JavaScript:node --checkrejected tests/tests/src/conditional/cond_a.mjs ("Export 'u' is not defined in module") and cond_a_none.mjs ("Export 'A' is not defined in module").Note for review: The fix is a final pass in
Lam_compile_mainthat declares exported names left unbound when the module body always throws; the alternative would be to change howPraisecompiles aDeclarecontinuation.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