Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we have a whole bunch of ways to represent types:
rose::Type
rose::Typexpr
(accompanied byrose::id::Typexpr
)rose::Typedef
(accompanied byrose::id::Typedef
)rose::TypeNode
rose_interp::Typeval
This PR replaces it all with just
rose::Ty
(accompanied byrose::id::Ty
).The
Typedef
andTypeNode
stuff came from the idea that in order to do #34 we'd need to have our graph of objects include not just functions but also typedefs (which may include generics of their own), so that we can attach metadata about field names to those (for structs which we represent as tuples). But after doing more implementation work, it's become clear that most functions using these typedefs are going to need to pull in parts of their definitions into their own list of types, in order for their local variables to talk about the fields of those structs. The end result is just a lot of extra complexity added with no real memory usage benefit.In contrast, when we remove typedefs (obviating #47), we now have the inductive property that, if the representations of two types in a function definition are equal, that is exactly equivalent to those types being compatible with each other in our type system, making #28 much easier. Of course this only works if we deduplicate types as we add them, for which purpose this PR pulls in the
indexmap
crate.That covers the reason for the other bloat in representations: previously we were using
Type
vsTypexpr
to distinguish "simple" from "more complicated" types, specifically in the sense that the latter were "type constructors" made up of smaller types, whereas the former were made up of other things but not types. It seemed silly to include many duplicates of primitive types likeUnit
andBool
in our list of defined types, soType
wasCopy
to avoid that. But now we're deduplicating all types, not just primitives, so there's no need to separate these two categories. This eliminates more complexity in code that would have liked to treat all types the same, more of which was being added in #51 and #53.One question which then comes up is, how should we achieve #34? If we don't have
Typedef
and we're deduplicating types as we add them to a function definition, how can we attach metadata about field names? One way could be to define a newenum
specifically for representing types in therose-web
crate, which distinguishes between tuples representing function argument lists and tuples representing structs; the latter would store field names, perhaps usingRc<str>
. Then storing those in anIndexSet
would only deduplicate at the semantic level ofrose-web
, and when webake
the final function, we can keep around any information about field names that we need for the function signature, and otherwise just map over our list of types and get rid of the field name info to construct actual Rose IR. In particular this means that when we write the validator, even if it deduplicates types as part of its checking routine, it should not enforce that the types are already deduplicated, because use cases like this show that sometimes it's more convenient not to deduplicate.