Skip to content

Commit

Permalink
properly fixed longstanding bug in typer related to records and list …
Browse files Browse the repository at this point in the history
…expressions that contained projections and get operations.
  • Loading branch information
jsonch committed Aug 27, 2024
1 parent cdaa003 commit 53f62db
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
23 changes: 13 additions & 10 deletions src/lib/frontend/typing/Typer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -254,18 +254,21 @@ let rec infer_exp (env : env) (e : exp) : env * exp =
e'.espan
"Cannot dynamically create tuples containing global types"
else (
(* WARNING: This is a hack that needs to be fixed if users can write tuples.
If we have a tuple that contains a get operation to another tuple,
the effect of the get op will be a projection to the element
of the inner tuple. In the current representation I don't think there's a
way to unify something like "the effect of the 5th element of the outer tuple is
equal to the 2nd element of some other tuple". And expressions like this can
be generated by the compiler -- e.g., when recordElimination turns records
into tuples. *)
let expected = match (e'.e) with
(* This commented out code was a workaround to get unification to work
for tuple expressions that contained elements from other tuples.
The problem was that: tup foo = (bar.1, bar.2) failed to unify,
because it tried to unify the effect of foo.0 with bar.1, and there
so it ended up trying to unify FSucc(FProj()) with FProj()
The solution was to add a case in TyperUnify.try_unify_effect:
| FProj(FVar(tqv)), eff | eff, FProj(FVar(tqv)) -> ...
This solves the problem because in such a case, one of the sides
is going to be a variable. And it is safe because projecting
does not have an effect itself. *)
(* let expected = match (e'.e) with
| EOp(TGet(_, idx), _) -> wrap_effect tuple_eff [None, 0; None, idx]
| _ -> wrap_effect tuple_eff [None, 0; None, i]
in
in *)
let expected = wrap_effect tuple_eff [None, 0; None, i] in
let derived = (Option.get e'.ety).teffect in
unify_effect e.espan expected derived
)
Expand Down
10 changes: 10 additions & 0 deletions src/lib/frontend/typing/TyperUnify.ml
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ let rec try_unify_effect span eff1 eff2 =
eff
| FZero, FZero -> ()
| FSucc eff1, FSucc eff2 | FProj eff1, FProj eff2 -> try_unify eff1 eff2
(* unwrap a projection of a variable, because projection itself does not change effect *)
(* note: this has to come after the base FProj case. *)
| FProj(FVar(tqv)), eff | eff, FProj(FVar(tqv)) ->
try_unify_tqvar
(occurs_effect span)
try_unify
FTQVar.phys_equiv_tqvar
tqv
eff

| FIndex (id1, eff1), FIndex (id2, eff2) ->
if not (Id.equal id1 id2) then raise CannotUnify else try_unify eff1 eff2
| (FZero | FSucc _ | FProj _ | FIndex _), _ -> raise CannotUnify)
Expand Down

0 comments on commit 53f62db

Please sign in to comment.