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

Untagged variants: consider regexp as an object type. #6296

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

cristianoc
Copy link
Collaborator

Fixes #6140

CHANGELOG.md Outdated
@@ -16,6 +16,7 @@

- Introduced a new `%ffi` extension (*experimental* - not for production use!) that provides a more robust mechanism for JavaScript function interoperation by considering function arity in type constraints. This enhancement improves safety when dealing with JavaScript functions by enforcing type constraints based on the arity of the function. https://github.com/rescript-lang/rescript-compiler/pull/6251
- Extended untagged variants with function types. https://github.com/rescript-lang/rescript-compiler/pull/6279
- Untagged variants: consider regexp as an object type. https://github.com/rescript-lang/rescript-compiler/pull/6296
Copy link
Member

Choose a reason for hiding this comment

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

This needs to go to beta.3 actually, I will do the PR for preparing beta.3 after I have got the "publish to npm on tag build" stuff working.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll rebase.

@@ -117,7 +117,7 @@ let type_is_builtin_object (t : Types.type_expr) =
match t.desc with
| Tconstr (path, _, _) ->
let name = Path.name path in
name = "Js.Dict.t" || name = "Js_dict.t"
name = "Js.Dict.t" || name = "Js_dict.t" || name = "Js.Re.t" || name = "RescriptCore.Re.t"
Copy link
Contributor

Choose a reason for hiding this comment

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

For the cases when Js/Belt/RescriptCore opened globally:

Suggested change
name = "Js.Dict.t" || name = "Js_dict.t" || name = "Js.Re.t" || name = "RescriptCore.Re.t"
name = "Js.Dict.t" || name = "Js_dict.t" || name = "Dict.t" || "RescriptCore.Dict.t" || name = "Js.Re.t" || name = "Re.t" || name = "RescriptCore.Re.t"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's not how it works. Internally, you still get the full path.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2023-06-08 at 20 36 03

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm talking about the case when there's a custom implementation of a module from stdlib.
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it. That is another topic, which is pretty interesting, and worth discussing in a separate issue.

Quick thoughts: assuming it's what it is just based on name sounds risky, and perhaps not even necessary.

Topic: how to represent that a type is an object type.

  1. For untagged variants, it's useful to express more patterns
  2. For options, it helps as they can be represented unboxed
  3. For the language: the current mechanism {..} is both obscure and not very useful/flexible.

For points 1 and 2, one can already recognise when a type used is defined as an object type (unless I'm misremembering -- check).
For abstract types, one could e.g. use an annotation @object.

For 3, perhaps one could use a new built-in type object<_> with some operations on it. E.g. creation will return object<'a> for an arbitrary 'a.
There has to be a mechanism to cast actual objects into the type object<...>.
The Dict module might look very different if this is used.
So having an object<...> type is only one possibility. But if one embraces this possibility then the case of the abstract types would work like this:

type someSecretType
type someAbstractObject = object<someSecretType>

without the need for any @object annotation.

@cristianoc
Copy link
Collaborator Author

@DZakh would you check that this works for you (installing the package produced by CI when it's done).

@cristianoc cristianoc merged commit 6a9fe4a into master Jun 9, 2023
14 checks passed
@cristianoc cristianoc deleted the regexp_as_object branch June 9, 2023 14:47
@DZakh
Copy link
Contributor

DZakh commented Jun 9, 2023

@cristianoc I've tested. Works great ✨

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.

Untegged variants support for regular expressions as a payload
3 participants