Skip to content

Fix compiler crash for options of transitive dependency types - #8719

Open
fhammerschmidt wants to merge 2 commits into
rescript-lang:masterfrom
fhammerschmidt:fix/option-transitive-type-lookup
Open

fhammerschmidt wants to merge 2 commits into
rescript-lang:masterfrom
fhammerschmidt:fix/option-transitive-type-lookup

Conversation

@fhammerschmidt

Copy link
Copy Markdown
Member

Constructing Some(Facade.v) or passing Facade.v to an optional argument currently crashes with Fatal error: exception Not_found when its type is declared in a transitive dependency whose interface is absent from the consumer's search path.

Match directly on the type-declaration lookup so a missing declaration takes the conservative option-wrapping path. Known representations retain their existing behavior, and only Not_found from the lookup is caught.

Add a cross-module build regression that compiles the consumer with only the facade's include path, then executes the generated JavaScript. It covers variants, records, aliases, abstract types, unboxed types, optional arguments, and preservation of Some(undefined) versus None. Add focused unit coverage for unavailable declarations and known integer types.

Fixes #8687.

Validation:

  • The new build regression fails with Not_found before the fix and passes afterward.
  • make test passes, including formatting, 330 compiler unit tests, 558 runtime tests, build fixtures, and 741 documentation examples.

Signed-off-by: Florian Hammerschmidt <florianh89@gmail.com>
Signed-off-by: Florian Hammerschmidt <florianh89@gmail.com>
@cknitt
cknitt requested a review from cristianoc October 2, 2026 18:35
@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.03%. Comparing base (95bdbd1) to head (3898ccd).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
compiler/ml/typeopt.ml 66.66% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8719      +/-   ##
==========================================
+ Coverage   78.95%   79.03%   +0.08%     
==========================================
  Files         474      475       +1     
  Lines       64231    64180      -51     
==========================================
+ Hits        50711    50724      +13     
+ Misses      13520    13456      -64     
Files with missing lines Coverage Δ
tests/ounit_tests/ounit_tests_main.ml 100.00% <ø> (ø)
tests/ounit_tests/ounit_typeopt_tests.ml 100.00% <100.00%> (ø)
compiler/ml/typeopt.ml 82.69% <66.66%> (+3.44%) ⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 2, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8719

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@8719

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8719

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8719

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8719

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8719

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8719

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8719

commit: 3898ccd

@cknitt

cknitt commented Oct 4, 2026

Copy link
Copy Markdown
Member

@codex review

This branch has not been deployed

No deployments
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.

bsc crashes with Fatal error: exception Not_found on Some(x) when the type of x comes from a transitive dependency

2 participants