-
Notifications
You must be signed in to change notification settings - Fork 44
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
type assignment for internal expressions #1121
base: dev
Are you sure you want to change the base?
Conversation
…nto haz3l-properties
…nto haz3l-properties
…nto haz3l-properties
2efdb09
to
7f73d07
Compare
}) | ||
|> OptUtil.sequence; | ||
let ctx' = | ||
List.fold_left( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can probably combine the map above and this fold into one, but not critical
l2, | ||
); | ||
} else { | ||
ctx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to error out in cases like this
} else { | ||
ctx; | ||
}; | ||
| _ => ctx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use a wildcard here
| _ => None | ||
}; | ||
| ApBuiltin(_) | ||
| BuiltinFun(_) => None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to have a way of checking and looking up the type of a builtin, not just erroring on uses of builtins
None; | ||
}; | ||
| ListLit(_, _, ty, _) => Some(List(ty)) | ||
| Cons(d1, ListLit(_, _, _, [])) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this case for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ListLits have types so don't need to special case nil...
| _ => None | ||
}; | ||
| ListConcat(ListLit(_, _, _, []), d) | ||
| ListConcat(d, ListLit(_, _, _, [])) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
|
||
switch (typ_cases) { | ||
| None => None | ||
| Some(_) => Some(Typ.Unknown(Internal)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's actually check to make sure that there is at least one branch with an inconsistent type here
if (Typ.eq(ty, Bool)) { | ||
let* _ = typ_of_dhexp(ctx, m, d1); | ||
let* _ = typ_of_dhexp(ctx, m, d2); | ||
Some(Typ.Unknown(Internal)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here -- let's actually check to make sure that there is at least one branch with an inconsistent type
| ExpandingKeyword(_) | ||
| InvalidText(_) | ||
| BadConstructor(_) | ||
| IntLit(_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should type check in these cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good for a merge now, can you just remove the remaining prints that ended up in here?
Adds a module that performs type assignment after conducting elaboration.