Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add Parsetree-level Option stdlib optimizations (forEach/map/flatMap)#7918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
cknitt wants to merge3 commits intomaster
base:master
Choose a base branch
Loading
fromoptions-stdlib-opt

Conversation

@cknitt
Copy link
Member

@cknittcknitt commentedSep 22, 2025
edited
Loading

This PR adds a compiler-side optimization for the most important functions from the standard library'sOption module:Option.forEach/map/flatMap.

  • At the Parsetree stage, we rewrite these into the same switch structure a handwritten version would yield, whenever the callback is either a literal lambda with a simple variable binder or an inlineable identifier. More complex callbacks stay as calls so evaluation order and side-effects remain untouched.
  • Because the rewrite runs ahead of type checking, the existing pipeline still insertsPrimitive_option.valFromOption exactly as before, so the generated JavaScript stays identical. The trade-off is that we can only recognize calls spelled explicitly asOption.* orStdlib_Option.*; aliases or locally opened variants aren’t rewritten.
  • Typedtree and Lambda remain untouched. Threading a rewrite through Typedtree would be far more invasive than we need, and once we reach Lambda the primitive-versus-boxed distinction this optimization relies on has already disappeared.
  • The change is covered by the comprehensive tests intests/tests/src/option_stdlib_optimization_test.res. The first test in that file gives a good example of how much the compiler output for long pipe chains is simplified.

@pkg-pr-new
Copy link

pkg-pr-newbot commentedSep 22, 2025
edited
Loading

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7918

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7918

@rescript/win32-x64

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

commit:50dbc3c

Copy link
Collaborator

@cristianoccristianoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is going to be unsafe: Option.x could be anything.
We won't know what moduleOption we're talking about until the typed tree.
Also, aliases will be resolved by then.
We already have code in typecore.ml (I think) that checks exactly that: is this id the path I expect from predefined types. (Or in this case I guess, from sodlib).

There's no big reason it should be much harder. Worst case one can type it, then figure out the optimisation, then go back to the untyped type tree, modify, and re-type.
That would be the emergency solution if it turns out to be too hard.

@cknitt
Copy link
MemberAuthor

@codex review

Copy link

@chatgpt-codex-connectorchatgpt-codex-connectorbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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 👍.

Comment on lines +35 to +46
let transform (expr : Parsetree.expression) : Parsetree.expression =
match expr.pexp_desc with
| Pexp_apply
{
funct =
{
pexp_desc =
Pexp_ident
{txt = Ldot (Lident ("Option" | "Stdlib_Option"), fname)};
};
args = [(_, opt_expr); (_, func_expr)];
} -> (

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

[P1] Guard rewrite against user-definedOption modules

The optimization currently rewrites any application whose head is syntacticallyOption.* orStdlib_Option.* (see theLdot (Lident ("Option" | "Stdlib_Option"), fname) pattern). Because this runs before name resolution, it will also trigger for locally defined modules namedOption orStdlib_Option. If a project defines its own module shadowing these names (for example a customOption.map with different semantics), the pass will silently replace the call with a match onSome/None, miscompiling the user’s code. The transformation needs to confirm it is targeting the stdlib Option module—e.g. by running after type checking or by explicitly checking the module path after resolution—otherwise it is unsound.

Useful? React with 👍 / 👎.

cknitt reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@cristianoccristianoccristianoc left review comments

@chatgpt-codex-connectorchatgpt-codex-connector[bot]chatgpt-codex-connector[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cknitt@cristianoc

[8]ページ先頭

©2009-2025 Movatter.jp