Skip to content

Commit

Permalink
Speed up record inclusion check.
Browse files Browse the repository at this point in the history
Speed up record inclusion check.
Fixes #6284

Record inclusion check (between implementation and interface) is quadratic.

Example:
```res
module M : {
  type t<'a, 'b, 'c> = {x:list<('a, 'b)>, y:int, z:int}
} = {
  type t<'a, 'b, 'c> = {x:list<('a, 'c)>, y:int, z:int}
}
```

The algorithm tries to instantiate type parameters. It only reports an error if there is an inconsistency. This requires solving type equations involving many types at once.

To improve error message, the first problematic field is reported. So the type equations are checked again and again with size 1, 2, ...n where n is the number of fields. (Plus the type parameters).

This is quadratic and is problematic for types of ~1K elements.

This PR provides a fast path which just checks if there is an error, without blaming a specific field.
The fast path is linear.
Only if an error is detected, the quadratic path is take to blame precisely which field is involved.
  • Loading branch information
cristianoc committed Jun 7, 2023
1 parent 3ad5cb1 commit cda4816
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

- Fix issue where uncurried type internals leak in type error. https://github.com/rescript-lang/rescript-compiler/pull/6264
- Improve error messages for untagged variant definitions https://github.com/rescript-lang/rescript-compiler/pull/6290

- Fix type checking performance issue for large records. https://github.com/rescript-lang/rescript-compiler/issues/6284

# 11.0.0-beta.1

Expand Down
34 changes: 32 additions & 2 deletions jscomp/ml/includecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,35 @@ and compare_variants ~loc env params1 params2 n
end


and compare_records_fast ~loc env params1 params2 n
(labels1 : Types.label_declaration list)
(labels2 : Types.label_declaration list) =
match labels1, labels2 with
[], [] ->
Ctype.equal env true params1 params2
| [], _::_ -> false
| _::_, [] -> false
| ld1::rem1, ld2::rem2 ->
if Ident.name ld1.ld_id <> Ident.name ld2.ld_id
then false
else if ld1.ld_mutable <> ld2.ld_mutable then false else begin
Builtin_attributes.check_deprecated_mutable_inclusion
~def:ld1.ld_loc
~use:ld2.ld_loc
loc
ld1.ld_attributes ld2.ld_attributes
(Ident.name ld1.ld_id);
let field_mismatch = !Builtin_attributes.check_bs_attributes_inclusion
ld1.ld_attributes ld2.ld_attributes
(Ident.name ld1.ld_id) in
match field_mismatch with
| Some _ -> false
| None ->
compare_records_fast ~loc env
(ld1.ld_type::params1) (ld2.ld_type::params2)
(n+1)
rem1 rem2
end
and compare_records ~loc env params1 params2 n
(labels1 : Types.label_declaration list)
(labels2 : Types.label_declaration list) =
Expand Down Expand Up @@ -324,8 +353,9 @@ let type_declarations ?(equality = false) ~loc env name decl1 id decl2 =
if equality then mark cstrs2 Env.Positive (Ident.name id) decl2;
compare_variants ~loc env decl1.type_params decl2.type_params 1 cstrs1 cstrs2
| (Type_record(labels1,rep1), Type_record(labels2,rep2)) ->
let err = compare_records ~loc env decl1.type_params decl2.type_params
1 labels1 labels2 in
let ok = compare_records_fast ~loc env decl1.type_params decl2.type_params 1 labels1 labels2 in
let err = if ok then [] else
compare_records ~loc env decl1.type_params decl2.type_params 1 labels1 labels2 in
if err <> [] || rep1 = rep2 then err else
[Record_representation (rep1, rep2)]
| (Type_open, Type_open) -> []
Expand Down

0 comments on commit cda4816

Please sign in to comment.