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

Allow type variables when spreading record type definitions #6309

Merged
merged 14 commits into from
Jun 25, 2023

Conversation

cristianoc
Copy link
Collaborator

@cristianoc cristianoc commented Jun 22, 2023

Allow using type variables when spreading record type definitions.

Check this out for an example of what now works: https://github.com/rescript-lang/rescript-compiler/blob/ad9e0319e59b083f75134739b84b9c2c1dc42218/jscomp/test/record_type_spread.res

jscomp/ml/typedecl.ml Outdated Show resolved Hide resolved
jscomp/ml/record_spread.ml Outdated Show resolved Hide resolved
jscomp/ml/record_spread.ml Outdated Show resolved Hide resolved
jscomp/ml/typedecl.ml Outdated Show resolved Hide resolved
jscomp/ml/typedecl.ml Outdated Show resolved Hide resolved
jscomp/ml/typedecl.ml Outdated Show resolved Hide resolved
jscomp/ml/typedecl.ml Outdated Show resolved Hide resolved
@cristianoc
Copy link
Collaborator Author

This example seems to still have some problems:

// tst.res

type t<'a, 'b> = {
  x: result<'a, 'b>,
}

type d = {
  ...t<int, int>,
}

gives

% ./bsc tst.res

  We've found a bug for you!
  tst.res:6:1-8:1

   4 │ }
   5 │ 
   6 │ type d = {
   7 │   ...t<int, int>,
   8 │ }
   9 │ 
  10 │ 

  The implementation tst.res
  does not match the interface (inferred signature):
  Type declarations do not match:
    type d = {x: result<int, int>}
  is not included in
    type d = {x: result<int, int>}
  tst.res:6:1-8:1: Expected declaration
  tst.res:6:1-8:1: Actual declaration
  The types for field x are not equal.

@cristianoc
Copy link
Collaborator Author

This example seems to still have some problems:

// tst.res

type t<'a, 'b> = {
  x: result<'a, 'b>,
}

type d = {
  ...t<int, int>,
}

gives

% ./bsc tst.res

  We've found a bug for you!
  tst.res:6:1-8:1

   4 │ }
   5 │ 
   6 │ type d = {
   7 │   ...t<int, int>,
   8 │ }
   9 │ 
  10 │ 

  The implementation tst.res
  does not match the interface (inferred signature):
  Type declarations do not match:
    type d = {x: result<int, int>}
  is not included in
    type d = {x: result<int, int>}
  tst.res:6:1-8:1: Expected declaration
  tst.res:6:1-8:1: Actual declaration
  The types for field x are not equal.

It fails here (with debug printing in includecore):

          let current_field_consistent =
            let eq = Ctype.equal env true [ld1.ld_type] [ld2.ld_type] in
            if not eq then
            (
              Format.eprintf "ld1.ld_type:\n%a\nld2.ld_type:\n%a\n"
                Printtyp.raw_type_expr ld1.ld_type Printtyp.raw_type_expr ld2.ld_type;
              assert false
            );

Giving:

ld1.ld_type:
{id=488;level=100000000;desc=
              Tconstr(result,
               [{id=916;level=1021;desc=Tconstr(int,[],[])};
                {id=919;level=1021;desc=Tconstr(int,[],[])}],[result])}
ld2.ld_type:

{id=983;level=100000000;desc=
 Tconstr(result,
  [{id=984;level=100000000;desc=Tconstr(int,[],[])};
   {id=985;level=100000000;desc=Tconstr(int,[],[])}],[result])}

@zth zth changed the title Trying to figure out record type expansion with type variables. Allow type variables when spreading record type definitions Jun 25, 2023
@zth zth merged commit 508e2b3 into master Jun 25, 2023
2 checks passed
@zth zth deleted the explore-type-params-in-record-spreads branch June 25, 2023 10:24
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