Skip to content

Resolve FFI externals during type checking - #8728

Draft
cknitt wants to merge 1 commit into
masterfrom
codex/resolve-external-in-typedecl
Draft

cknitt wants to merge 1 commit into
masterfrom
codex/resolve-external-in-typedecl

Conversation

@cknitt

@cknitt cknitt commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Why

Parsetree is the AST that PPXs see, and we want to freeze a version of it as "v1" at some point (#8624). Before that, it should only describe source code.

Externals didn't. After the frontend processed the FFI attributes of an external, it replaced the primitive string with the computed FFI spec (Prim_ffi, Prim_inline_const). As a result:

  • parsetree.ml depended on External_ffi_types, the compiler's internal FFI representation. That type would become part of any frozen AST, and of a standalone syntax package we'd like to publish on opam.
  • The AST type had cases a PPX can never see, because PPXs run before the frontend. The v0 bridge needed an error branch for them.

What changes

pval_prim is now just the string from the source, with its location (string loc option). For @send external join: … = "join" it is "join"; the @send stays an attribute.

The FFI attributes are now interpreted during type checking. Typedecl calls Primitive.resolve_external, which the frontend registers, since that code lives above compiler/ml. The type checker ends up with the same types and primitive descriptions as before, so the generated JS is unchanged.

The frontend still processes each external once, for two reasons:

  • errors and warnings stay where they were;
  • it needs to know whether the external uses a relative @module path, which stops other modules from inlining it.

It now leaves the declaration as written, though. The type checker's second pass runs with warnings off, so nothing is reported twice.

Smaller changes that follow from this:

  • @inline constants used to be encoded as Prim_inline_const. They are now external x: T = "#rescript-inline" with the @inline(<literal>) attribute kept.
  • The unused-attribute check now looks at the processed form of each external rather than the declaration as written.
  • The outcome printer (hover, interface output) gets its own small type for processed primitives, Outcometree.out_primitive.
  • Typedtree.val_prim is a string as well; gentype now reads the external's name from the type checker's primitive description.
  • The AST and CMT magic numbers are bumped.

Tests

  • New ast-mapping fixture with several kinds of externals, all printing back unchanged after the v0 round trip.
  • New ounit test for an external's string and attributes surviving the v0 round trip.
  • New super_errors fixture for unused attributes on an external's arguments, which are now checked on the processed form, in source order.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.28571% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.04%. Comparing base (4e11b25) to head (0527e03).

Files with missing lines Patch % Lines
compiler/frontend/ast_external.ml 93.33% 2 Missing ⚠️
compiler/frontend/ast_external_mk.ml 89.47% 2 Missing ⚠️
compiler/frontend/bs_builtin_ppx.ml 88.88% 2 Missing ⚠️
compiler/ml/primitive.ml 60.00% 2 Missing ⚠️
compiler/frontend/bs_ast_invariant.ml 87.50% 1 Missing ⚠️
compiler/ml/printast.ml 0.00% 1 Missing ⚠️
compiler/ml/printtyped.ml 0.00% 1 Missing ⚠️
compiler/syntax/src/res_outcome_printer.ml 66.66% 1 Missing ⚠️
compiler/syntax/src/res_printer.ml 50.00% 1 Missing ⚠️
tests/ounit_tests/ounit_ast_mapper0_tests.ml 94.73% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8728      +/-   ##
==========================================
+ Coverage   81.01%   81.04%   +0.02%     
==========================================
  Files         454      454              
  Lines       63187    63191       +4     
==========================================
+ Hits        51193    51214      +21     
+ Misses      11994    11977      -17     
Files with missing lines Coverage Δ
compiler/core/lam_compile_external_call.ml 92.76% <ø> (ø)
compiler/ext/config.ml 100.00% <ø> (ø)
compiler/frontend/ast_attributes.ml 94.52% <100.00%> (+2.73%) ⬆️
compiler/frontend/ast_exp_handle_external.ml 82.50% <100.00%> (ø)
compiler/frontend/ast_external_process.ml 77.51% <100.00%> (-0.15%) ⬇️
compiler/frontend/ppx_entry.ml 91.66% <100.00%> (ø)
compiler/gentype/translation.ml 92.85% <100.00%> (+1.42%) ⬆️
compiler/ml/ast_iterator.ml 94.15% <100.00%> (+0.01%) ⬆️
compiler/ml/ast_mapper.ml 98.42% <100.00%> (+<0.01%) ⬆️
compiler/ml/ast_mapper_from0.ml 77.14% <100.00%> (+0.02%) ⬆️
... and 17 more

... and 1 file 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.

@cknitt
cknitt force-pushed the codex/resolve-external-in-typedecl branch from 36bd95a to b71e398 Compare October 7, 2026 04:59
@pkg-pr-new

pkg-pr-new Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

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

@rescript/belt

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: 0527e03

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

`pval_prim` is the primitive string as written, with its location. The
frontend still checks each FFI external, but `Typedecl` gets its spec through
`Primitive.resolve_external`, so the parsetree no longer depends on
`External_ffi_types`.

Review: Why does the frontend resolve each external at all? It reports the
errors and warnings where they were reported before, and decides whether a
relative `@module` keeps other modules from inlining the external. The type
checker's second resolution runs without warnings.

Review: How do `@inline` constants work without `Prim_inline_const`? The
built-in PPX turns them into an external with the internal primitive
`#rescript-inline` that keeps the `@inline(<literal>)` attribute.

Review: Is the global `Primitive.resolve_external` safe with parallel
requests? `Ast_external` sets it once during module initialization, like the
`Oprint` hooks, and it is only read afterwards.

Area: frontend typechecker syntax gentype tests
Output: unchanged, generated JS of the test suites is byte-identical
Formatting: unchanged
Formats: ast cmt
PPX: unchanged, an external's primitive string reaches v0 as before
Editor: unchanged
Errors: none, the new fixture follows the warning_<NN> naming convention
Tests: +tests/syntax_tests/data/ast-mapping/Externals.res
 +tests/build_tests/super_errors/fixtures/warning_101_external_arg_attribute.res
 ~tests/ounit_tests/ounit_ast_mapper0_tests.ml
 ~tests/ounit_tests/ounit_string_literal_tests.ml
PR: #8728
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018oLiCQjGFSE2qwfYqCMF9e
Signed-off-by: Christoph Knittel <ck@cca.io>
@cknitt
cknitt force-pushed the codex/resolve-external-in-typedecl branch from 17bf46b to 0527e03 Compare October 16, 2026 19:29
@github-actions

Copy link
Copy Markdown

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.

1 participant