Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove caml_external_polyfill and the related behavior (e.g. ?primitive) #6925

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

cometkim
Copy link
Member

@cometkim cometkim commented Jul 29, 2024

Removed the feature introduced from #1361 and #3629

external fn: type = "?fn"

if the external name starts with ? then the compiler injects runtime resolver, which uses globalThis very dangerously.

The feature is unused and should be replaced by @scope or @module in user code.

Further works

There are still many ?-prefixed primitives, but actually specialized and don't accept user polyfill just like other % or # primitives.

If we remove caml runtimes, keeping it as a different syntax no longer makes sense IMHO. I'll work on refactoring the primitives in a separate PR.

@cometkim cometkim requested review from cristianoc, cknitt and zth July 31, 2024 18:54
@cometkim cometkim marked this pull request as ready for review July 31, 2024 18:54
@cometkim cometkim changed the title wip: drop user polyfill syntax on external Drop polyfill syntax on external (e.g. ?ffi) Jul 31, 2024
@cometkim cometkim force-pushed the drop-polyfill branch 2 times, most recently from dfa2144 to b4fcf89 Compare July 31, 2024 18:57
@cometkim

This comment was marked as resolved.

@cometkim cometkim changed the title Drop polyfill syntax on external (e.g. ?ffi) Remove caml_external_polyfill and the related behavior (e.g. ?primitive) Jul 31, 2024
@cometkim cometkim merged commit 7e8b601 into rescript-lang:master Aug 1, 2024
20 checks passed
@cometkim cometkim deleted the drop-polyfill branch August 1, 2024 08:44
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.

3 participants