More generic contract generation #1192
Conversation
There was a problem hiding this comment.
It's not urgent, but looking at code each time we add a new primop, it really feels cumbersome to do (add stuff to the lexer, to the parser, implement boiler plate code for dynamically typing the arguments, extract them, oh should I take ownership with match_shared! or not 🤷♂️ ). It's not urgent but there must be a better way of organizing (and splitting, as well) eval::operation.
| /// Data about type variables that is needed for polymorphic contracts to decide which actions to take. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct TypeVarData { | ||
| pub polarity: Polarity, |
There was a problem hiding this comment.
I guess this field will take on more values once you do the "one-pass" version? Morally, positive, negative, and the composite partial identities (positive, negative) and (negative, positive) ?
There was a problem hiding this comment.
I was expecting to need more data for type variables, indeed. Originally I was also tracking the variable kind, but that ended up being unnecessary for now.
src/label.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl HasUnifType for TypeVarData { |
There was a problem hiding this comment.
I find the name a tad misleading, I would expect a trait named HasUnifType to define a method has_unif_type(&self) -> bool. I don't know of a good name scheme for those kind of * -> * functions from a type to a type. AssociatedUnifType? Looks a bit long. ReifyAsUnifType? ReifyAsNickelType? I like reify, it sounds professional 😅
There was a problem hiding this comment.
+1 for reify 👑 Another possibility would be AsUnifType. Really, the problem seems to be that TypeVarData::unif_type() should actually be an associated constant; and that the trait can't be named UnifType to conform to the usual naming format.
There was a problem hiding this comment.
Right, I would also have called it UnifType naturally.
There was a problem hiding this comment.
I've renamed it to ReifyAsUnifType. Yell if that's a bad idea 😅
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
This avoids a pass through the merging code path. The result is a ~10% performance improvement on a large tf-ncl AWS example. Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
This PR introduces a type variable environment into labels and makes the tracking type variable polarities for contract generation dynamic. Additionally, it introduces two new primops to manipulate this environment.
This is preparatory work for #1177.