Conversation
# Conflicts: # src/impls.rs
Stack overflow in derive
Robbepop
left a comment
There was a problem hiding this comment.
All in all looks good to me. I have 2 important additions that we could do as follow-up or directly integrate into this PR since they introducing massively breaking and disruptive changes.
src/impls.rs
Outdated
| fn type_info() -> Type { | ||
| TypeComposite::new("BTreeMap", Namespace::prelude()) | ||
| .type_params(tuple_meta_type![(K, V)]) | ||
| .fields(Fields::named().field_of::<[(K, V)]>("elems")) |
There was a problem hiding this comment.
I have had a chat with Jaco about conceptualizing encoded structures. E.g. for mappings we define a concept that is that types are mappings if they are a sequence over a type that has a key and a value field. So the decoder side (JS API) could then query for such conceptual types and deduce that they are seen as mappings and provide semantics control for them. In this case we could say that BTreeMap is really just a sequence over a custom type (that we define) that simple has a key and value field for K and V respectively. The custom type could be:
pub struct KeyValue<K, V> {
key: K,
value: V,
}| fn type_info() -> Type { | ||
| TypeVariant::new("Option", Namespace::prelude()) | ||
| .type_params(tuple_meta_type![T]) | ||
| .variants( | ||
| Variants::with_fields() | ||
| .variant_unit("None") | ||
| .variant("Some", Fields::unnamed().field_of::<T>()), | ||
| ) | ||
| .into() |
There was a problem hiding this comment.
Seeing this we still have the problem that our encoding is not generics-aware which it should imo. So we end up having a different type definition for each Option<T> with a different T instead of re-using the one generic definition. I have made some thoughts about this and came to the conclusion that it should even be fairly simple to actually implement this with some additional built-ins.
There are actually 2 ways we could implement this:
By introducing two new built-in type classes generic_type and generic_param:
{
"strings": {
"Option", # ID 1
"Some", # ID 2
"None", # ID 3
"T", # ID 4
"Foo", # ID 5
"Bar", # ID 6
"Baz", # ID 7
}
"types": {
{ # ID 1
"variant": {
"path" [ 1 ],
"params": [ 4 ]
"variants": [
{
"name": [ 2 ],
"type": [ 2 ],
},
{
"name": [ 3 ],
}
],
}
},
{ # ID 2
# Generic parameter with name `"T"`.
"generic_param": {
"name": 4
}
},
{ # ID 3
"primitive": "u32"
},
{ # ID 4
"primitive": "bool"
},
{ # ID 5
# Forwards to the `Option<u32>` type.
"generic_type": {
"type": 1,
"params": 3,
}
},
{ # ID 6
# Demonstrates usage of a type that contains a Option<u32> type.
"composite": {
"path": [ 5 ],
"fields": [
{
"type": 5
}
],
}
},
{ # ID 7
# Forwards to the `Option<bool>` type.
"generic_type": {
"type": 1,
"params": 4,
}
},
{ # ID 8
# Demonstrates usage of a type that contains a Option<bool> type.
"composite": {
"path": [ 6 ],
"fields": [
{
"type": 7
}
],
}
},
{ # ID 9
# Demonstrates usage of a non-generic type.
"composite": {
"path": [ 7 ],
"field": [
{
"type": 3,
},
{
"type": 4,
},
]
}
}
}
}
By introducing flags for all type fields:
{
"strings": {
"Option", # ID 1
"Some", # ID 2
"None", # ID 3
"T", # ID 4
"Foo", # ID 5
"Bar", # ID 6
"Baz", # ID 7
}
"types": {
{ # ID 1
"variant": {
"path" [ 1 ],
"params": [ 4 ]
"variants": [
{
"name": [ 2 ],
"type": [
{
# 4 indexes into the string table and must have
# the same index as one of the identifier in `params`.
"parameter": 4
}
],
},
{
"name": [ 3 ],
}
],
}
},
{ # ID 3
"primitive": "u32"
},
{ # ID 4
"primitive": "bool"
},
{ # ID 5
# Demonstrates usage of a type that contains a Option<u32> type.
"composite": {
"path": [ 5 ],
"fields": [
{
# Demonstrates usage of the generic type at index 1 (`Option<T>`)
# with a parameter of 4 where 4 indexes into the type table as `u32`.
"type": {
"generic": {
"type": 1,
"params": 3,
},
}
}
],
}
},
{ # ID 6
# Demonstrates usage of a type that contains a Option<bool> type.
"composite": {
"path": [ 6 ],
"fields": [
{
"type": {
# Demonstrates usage of the generic type at index 1 (`Option<T>`)
# with a parameter of 4 where 4 indexes into the type table as `bool`.
"generic": {
"type": 1,
"params": 4,
},
}
}
],
}
},
{ # ID 7
# Demonstrates usage of a non-generic type.
"composite": {
"path": [ 7 ],
"field": [
{
"type": {
# 3 indexes into the type table and refers to a non
# generic (aka concrete) type, in this case `u32`.
"concrete": 3
},
},
{
"type": {
# 4 indexes into the type table and refers to a non
# generic (aka concrete) type, in this case `bool`.
"concrete": 4
}
},
]
}
}
}
}
src/impls.rs
Outdated
| fn type_info() -> Type { | ||
| TypeVariant::new("Option", Namespace::prelude()) | ||
| TypeVariant::new() | ||
| .path(Path::new().ident("Option")) |
There was a problem hiding this comment.
I'd rather go for a special constructor so that users have to opt-in to create a Voldemord type which should be a rather rare thing instead of making users opt-in to create a common named type.
There was a problem hiding this comment.
E.g. we could have 4 constructors
Path::voldemordPath::prelude(with prelude namespace)Path::new(name, namespace)(or so)
There was a problem hiding this comment.
And maybe Path::prelude should actually be hidden from users.
Migrated from type-metadata/type-metadata#42
TypeIdtoTypeand merges inTypeDefHasTypeDeftrait etc.Composite(for structs) andVariant(for enums) for user defined typesFieldwith optional name. Builders enforce construction of either all named (structs) or all unnamed (tuples).Variantwith optional fields. For "C-like" enums, a builder enforces that all variants have no fields and an optional discriminant. For enums with fields, we reuse theFieldtype and builders for "struct" and "tuple" variants.Downstream PRs
todo
TypeIdstruct?ink!comments from legacy PR
pathor not Merge type def into custom type id type-metadata/type-metadata#42 (comment) (keep it flat)