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

Print variant runtime repr in error messages, and fix inclusion check. #6669

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

cristianoc
Copy link
Collaborator

@cristianoc cristianoc commented Mar 7, 2024

  • Print @as(...) as required for variant cases.
  • Print @unboxed in the variant type declaration in the outcome printer.
  • Fix issue where attributes such as @unboxed were printed twice.
  • Fix issue where inconsistency in @unboxed in variant declarations between implementation and interface was not checked, and the type error was missing.

Test:

module M: {
  @unboxed type t = | @as(null) A
} = {
  @unboxed type t = | @as(undefined) A
}

Triggered in error message:

Type declarations do not match:
    @unboxed type t = @as(undefined) A
  is not included in
    @unboxed type t = @as(null) A

@cristianoc cristianoc changed the base branch from 11.0_release to master June 13, 2024 08:14
As in `type t = @as(undefined) A`

Triggered in error message:

```res
Type declarations do not match:
    type t = @as(undefined) A
  is not included in
    type t = @as(null) A
```
- Print `@unboxed` in the variant type declaration in the outcome printer.
- Fix issue where attributes such as `@unboxed` were printed twice.
- Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked.
@cristianoc
Copy link
Collaborator Author

Original issue here: rescript-lang/rescript-vscode#922

@cristianoc cristianoc requested review from cknitt and zth June 13, 2024 09:17
@cknitt
Copy link
Member

cknitt commented Jun 13, 2024

This is great! Thanks a lot!

Could you

  • add tests
  • add a CHANGELOG entry
  • remove "POC" from the issue title if this is otherwise ready to be merged

?

@cristianoc
Copy link
Collaborator Author

CI caught a bug in Nullable in core (while building the playground) where the type is missing the @unboxed annotation in the implementation. Cc @zth

@cknitt
Copy link
Member

cknitt commented Jun 13, 2024

Also, when building the playground, the following error occurs in CI:

FAILED: src/Core__Nullable.cmj

  We've found a bug for you!
  /home/runner/work/***-compiler/***-compiler/packages/playground-bundling/node_modules/@***/core/src/Core__Nullable.res:1:1-4:28

  1 │ type t<'a> = Js.Nullable.t<'a> =
  2 │   | Value('a)
  3 │   | @as(null) Null
  4 │   | @as(undefined) Undefined
  5 │ 
  6 │ external null: t<'a> = "#null"

  This variant or record definition does not match that of type
    Js.Nullable.t<'a>
  Their internal representations differ:
  the original definition uses unboxed representation.

@cknitt
Copy link
Member

cknitt commented Jun 13, 2024

Ah sorry, didn't see that you had already commented about that.

@cristianoc cristianoc changed the title POC print variant runtime repr in error messages Print variant runtime repr in error messages, and fix inclusion check. Jun 13, 2024
@cristianoc
Copy link
Collaborator Author

@cknitt @zth CI is happy now

Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

🎉

@cristianoc cristianoc merged commit 95c52fd into master Jun 14, 2024
19 checks passed
@cristianoc cristianoc deleted the variant_runtime_repe branch June 14, 2024 13:19
cknitt pushed a commit that referenced this pull request Jun 15, 2024
#6669)

* POC print variant runtime repr

As in `type t = @as(undefined) A`

Triggered in error message:

```res
Type declarations do not match:
    type t = @as(undefined) A
  is not included in
    type t = @as(null) A
```

* Print @unboxed for variants and check inclusion correctly.

- Print `@unboxed` in the variant type declaration in the outcome printer.
- Fix issue where attributes such as `@unboxed` were printed twice.
- Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked.

* snake case

* Changelog and test.

* Bump rescript-core for playground bundling.
cknitt pushed a commit that referenced this pull request Jun 16, 2024
#6669)

* POC print variant runtime repr

As in `type t = @as(undefined) A`

Triggered in error message:

```res
Type declarations do not match:
    type t = @as(undefined) A
  is not included in
    type t = @as(null) A
```

* Print @unboxed for variants and check inclusion correctly.

- Print `@unboxed` in the variant type declaration in the outcome printer.
- Fix issue where attributes such as `@unboxed` were printed twice.
- Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked.

* snake case

* Changelog and test.

* Bump rescript-core for playground bundling.
cknitt pushed a commit that referenced this pull request Jun 16, 2024
#6669)

* POC print variant runtime repr

As in `type t = @as(undefined) A`

Triggered in error message:

```res
Type declarations do not match:
    type t = @as(undefined) A
  is not included in
    type t = @as(null) A
```

* Print @unboxed for variants and check inclusion correctly.

- Print `@unboxed` in the variant type declaration in the outcome printer.
- Fix issue where attributes such as `@unboxed` were printed twice.
- Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked.

* snake case

* Changelog and test.

* Bump rescript-core for playground bundling.
cknitt pushed a commit that referenced this pull request Jun 16, 2024
#6669)

* POC print variant runtime repr

As in `type t = @as(undefined) A`

Triggered in error message:

```res
Type declarations do not match:
    type t = @as(undefined) A
  is not included in
    type t = @as(null) A
```

* Print @unboxed for variants and check inclusion correctly.

- Print `@unboxed` in the variant type declaration in the outcome printer.
- Fix issue where attributes such as `@unboxed` were printed twice.
- Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked.

* snake case

* Changelog and test.

* Bump rescript-core for playground bundling.
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.

None yet

2 participants