Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 9, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

# 11.0.0-beta.3 (Unreleased)

#### :rocket: New Feature
- Untagged variants: consider regexp as an object type. https://github.com/rescript-lang/rescript-compiler/pull/6296

# 11.0.0-beta.2

#### :rocket: New Feature
Expand Down
2 changes: 1 addition & 1 deletion jscomp/ml/ast_untagged_variants.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Member

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
Member

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.

| _ -> false

let get_block_type ~env (cstr : Types.constructor_declaration) :
Expand Down