Repository navigation
Conversation
Codecov Report❌ Patch coverage is 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
🚀 New features to boost your workflow:
|
cknitt
force-pushed
the
codex/resolve-external-in-typedecl
branch
from
October 7, 2026 04:59
36bd95a to
b71e398
Compare
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-8728 |
This was referenced Oct 7, 2026
cknitt
force-pushed
the
codex/resolve-external-in-typedecl
branch
3 times, most recently
from
October 13, 2026 20:15
e2110a3 to
17bf46b
Compare
`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
force-pushed
the
codex/resolve-external-in-typedecl
branch
from
October 16, 2026 19:29
17bf46b to
0527e03
Compare
This branch has not been deployed
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.
Why
Parsetreeis 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.mldepended onExternal_ffi_types, the compiler's internal FFI representation. That type would become part of any frozen AST, and of a standalonesyntaxpackage we'd like to publish on opam.What changes
pval_primis now just the string from the source, with its location (string loc option). For@send external join: … = "join"it is"join"; the@sendstays an attribute.The FFI attributes are now interpreted during type checking.
TypedeclcallsPrimitive.resolve_external, which the frontend registers, since that code lives abovecompiler/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:
@modulepath, 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:
@inlineconstants used to be encoded asPrim_inline_const. They are nowexternal x: T = "#rescript-inline"with the@inline(<literal>)attribute kept.Outcometree.out_primitive.Typedtree.val_primis a string as well; gentype now reads the external's name from the type checker's primitive description.Tests
ast-mappingfixture with several kinds of externals, all printing back unchanged after the v0 round trip.super_errorsfixture for unused attributes on an external's arguments, which are now checked on the processed form, in source order.🤖 Generated with Claude Code