-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat: newtype for (non-map) collection types #5133
Conversation
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.
It would help me if we extend the text in docs/dev/TypeSystemRefresh.md
to further elaborate on how things work and why.
I think I would prefer having a minimum set of example programs and how we want to infer types for them, as a way of explaining the requirements of the type inference system.
Next to that, if there's passes that can be described as constraint solving problems, it would help me to know what the possible states (type expressions) of the nodes (variables/expression) are.
Also, it might be good to describe the 'space' of types. For example, the pre-types might live in a lattice, and full types might be a generalization where each lattice node is a set of types that all have the same runtime type.
Lastly, have you considered renaming 'type adjustment' to 'type refinement' ? My impression is that this is a transformation that makes types strictly smaller, if you view them as sets of values.
Object | ||
} | ||
|
||
public abstract class Advice { |
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.
Could you add a comment to this class? What is advice for?
Description
This PR adds support for a
newtype
to useset
/iset
/seq
/multiset
as its base type. This includes type inference/checking, verification, and compilation to all targets. For example, one can now declare types likeusing
--type-system-refresh --general-newtypes
.A
newtype
defined in this way obtains new versions of the constructors of the base type. For example, just like the constructor7
ofint
also becomes a constructor for anewtype
-int
, themultiset{...}
becomes a constructor for anynewtype
-multiset
and"hello"
becomes a constructor fornewtype
-seq<char>
.Every built-in operator, say, of type
B * B -> B
, of the base typeB
becomes a built-in operator of typeN * N -> N
of thenewtype
-B
N
. For example,+
becomes a operator onString
above. As usual for newtypes, when the result of such an operator is a newtype, the constraint is checked.For these purposes, the subsequence expressions
s[_.._]
are operators (returning something of the same type ass
) if the type ofs
is a sequence, and are constructors (returning any sequence type) ifs
is an array type.Built-in functions, like
multiset(_)
, that take collections as inputs or outputs, are extended to work with newtypes as well.Other places where collections can be used (for example, frames and the argument to
fresh
) are extended to work with newtypes as well.Not including in this PR (but in upcoming PRs) are support for:
newtype
based onmap
newtype
with type parameters (but the base type can be a type with type parameters, provided they are filled in, likeseq<int>
in the example above)Comments for reviewers
In a great number of places, the change required to support these general newtypes was to change
.NormalizeExpand()
to.NormalizeToAncestorType()
. For example, this occurs frequently in contexts likeexpr.Type.NormalizeToAncestorType().AsSeqType
. It's possible that it would be better to change.AsSeqType
to do the.NormalizeToAncestorType()
internally (so that uses could look likeexpr.Type.AsSeqType
). However, I'd rather hold off on such changes until all the newtype work is done. At that time, we can better evaluate how much of the uses of.AsSeqType
need the ancestor type.Supporting the general newtypes during the type-adjustment phase required a new kind of flow,
FlowFromComputedTypeIgnoreHeadTypes
, as a variation of the existingFlowFromComputedType
. The new flow works on the ancestor of the sink type (for example, it applies toseq<int>
instead ofFourInts
).Arrow types are not supported as newtype base types (in fact, I'm wondering if they ever should be). This check was previously omitted.
I improved printing of arrow types in pre-type error messages. It might seem that subset types like
-->
and->
should never need to be printed during pre-type inference, but they can appear if the user input declares the types as such.I split up the
Advice
class into two subclasses,CommonAdvice
andTypeAdvice
. The latter supports types other than built-in parameter-less types.I improved pre-type inference to make non-committal use of advice earlier.
I fixed a bug (a glaring omission) in the
MeetHeads
method inPreTypeConstraints.cs
.Saying that a result type is a collection type or newtype-collection type requires some setup. I did a little refactoring so that this new setup can share code.
I fixed a bug in an error-message format string (that could cause a crash).
I fixed two bugs in the type inference of
-
and>
, where the result was not checked to be of an appropriate type.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.