-
Notifications
You must be signed in to change notification settings - Fork 194
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
Support for go generics #327
Comments
codegen + generics ... srsly? @mredolatti |
This would be a great change to have. I think having codegen for generic things makes a lot of sense when wanting to serialize structs wrapping arbitrarily-typed values as described in the issue. Perhaps it would be a requirement that the type parameter(s) are constrained by |
I took a very rough stab at supporting generics in this library https://github.com/tinylib/msgp/compare/master...matthinrichsen-wf:msgp:generics?expand=1 |
It is quite hard to judge without tests. I don't think we can go too wrong with it - since it just doesn't work now - but having a fairly solid "v1" release of it would of course be preferable. |
AFAICT the most difficult part is to support primitive types. Looking at the most basic example:
If we generate the methods with proper MarshalMsgTrivial. EncodeMsgTrivial. UnmarshalMsgNow the fun begins:
This will not work, since // ReadIntoIntfBytes will read a value and store it in i.
// The destination i must be a pointer, so the value is writable.
func ReadIntoIntfBytes(i any, b []byte) (o []byte, err error) {
switch i := i.(type) {
case Unmarshaler:
return i.UnmarshalMsg(b)
case Extension:
return ReadExtensionBytes(b, i)
case *bool:
*i, o, err = ReadBoolBytes(b)
return
case **bool:
if IsNil(b) {
*i = nil
return ReadNilBytes(b)
}
var dst bool
if dst, o, err = ReadBoolBytes(b); err != nil {
return nil, err
}
*i = &dst
case *float32:
*i, o, err = ReadFloat32Bytes(b)
//..
}
// DO reflection similar to AppendIntf.
return b, fmt.Errorf("cannot unmarshal into %T", i)
} There is probably also a need for The call would be DecodeMsgShould probably be similar to above. MsgSizeA function would need to be written that returns size of the interface type. Array |
The behavior for the current codegen when a type is opaque to the tool
(i.e. defined in a different source file) is to assume it implements
`Marshaler`, `Unmarshaler`, etc. I suspect it would be simplest just to
assume the same thing for `T any`. I suppose could even require any type
`T` for which we are generating code to explicitly satisfy the `Marshaler`
/ `Unmarshaler` / etc. constraints.
…On Wed, Aug 14, 2024 at 2:04 AM Klaus Post ***@***.***> wrote:
AFAICT the most difficult part is to support primitive types.
Looking at the most basic example:
type Foo[T any] struct {
X string
Bar T
}
If we generate the methods with proper Foo[T], we still have the problem
that we will have to provide functionality for any type of T. AFAICT this
means converting to an interface.
MarshalMsg
Trivial. o, err = msgp.AppendIntf(o, z.Bar) should encode it.
EncodeMsg
Trivial. err = en.WriteIntf(z.Bar) should write it.
UnmarshalMsg
Now the fun begins:
case "Bar":
bts, err = z.Bar.UnmarshalMsg(bts)
This will not work, since z.Bar doesn't guarantee the compiler that
UnmarshalMsg is provided. AFAICT we do not have a function that provides
this. As far as I can tell this should do it:
// ReadIntoIntfBytes will read a value and store it in i.// The destination i must be a pointer, so the value is writable.func ReadIntoIntfBytes(i any, b []byte) (o []byte, err error) {
switch i := i.(type) {
case Unmarshaler:
return i.UnmarshalMsg(b)
case Extension:
return ReadExtensionBytes(b, i)
case *bool:
*i, o, err = ReadBoolBytes(b)
return
case **bool:
if IsNil(b) {
*i = nil
return ReadNilBytes(b)
}
var dst bool
if dst, o, err = ReadBoolBytes(b); err != nil {
return nil, err
}
*i = &dst
case *float32:
*i, o, err = ReadFloat32Bytes(b)
//..
}
return b, fmt.Errorf("cannot unmarshal into %T", i)
}
There is probably also a need for reflect so *Unmarshaler would be
handled properly.
The call would be bts, err = msgp.ReadIntoIntfBytes(&z.Bar, bts).
DecodeMsg
Should probably be similar to above.
MsgSize
A function would need to be written that returns size of the interface
type.
Array []T and map types map[string]T also seems "interesting", and will
probably need some special care.
—
Reply to this email directly, view it on GitHub
<#327 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWN7ZQV5YNAYAHHGP4IRL3ZRMMS7AVCNFSM6AAAAABLA4IZ4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBYGIZTMMJXGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@philhofer The issue is that But yeah, that could be a reasonable first step, but it would be rather limited. |
I checked the asm output from this, and it doesn't seem like the compiler can deduce the type and the Branches will however be fully predictable, so it will not be the end of the world in terms of performance - but somewhat suboptimal. In terms of speed, the "foreign type" assumption seems fine. However, it does place big restrictions on the usage. Let's say we want a generic with a string type (but it could be any primitive).. We try:
We generate the output from this. Except for the missing However the issue then becomes that the non-pointer I hope this makes sense to why I was looking at an approach that involved annoying I may very well be missing something, but it seems like treating generic types as foreign types will significantly reduce the usability - at least for me. |
Hi! are there any plans to support generic structs?
Something like this:
will try to generate code as if
Response
was indeed a struct and instead of a template.Thanks!
The text was updated successfully, but these errors were encountered: