Skip to content
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

Make shim directive handle map key #345

Open
phqb opened this issue Apr 27, 2024 · 1 comment
Open

Make shim directive handle map key #345

phqb opened this issue Apr 27, 2024 · 1 comment

Comments

@phqb
Copy link

phqb commented Apr 27, 2024

I'm using tinylib/msgp to add marshaling/unmarshaling to existing structs. This library works very well in my usecase. I appreciate tinylib/msgp's maintainers a lot.

My structs have a lot of map whose keys are not string and MessagePack requires map key to be string. I can replace all these map keys with string but doing this makes my structs lost some of their readability and I have to do some refactoring.

For an example, I have to change the following struct:

type Foo <some type can convert to/from string> 

type MyStruct struct {
    Map map[Foo]*Bar
}

to

type MyStruct struct {
    MapByFoo map[string]*Bar
}

to maintain readability.

tinylib/msgp already has shim directive that allows any type to be (un)marshaled as another type. I use this feature a lot to avoid refactoring my structs. It will be nice that shim directive also transform non-string map key to string so the following code:

//msgp:shim Foo as:string using:FooToString/StringToFoo

type MyStruct struct {
    Map map[Foo]*Bar
}

will be generated as

// EncodeMsg implements msgp.Encodable
func (z *MyStruct) EncodeMsg(en *msgp.Writer) (err error) {
        ...
	for za0001, za0002 := range z.Map {
		err = en.WriteString(FooToString(za0001)) <-----
		if err != nil {
			err = msgp.WrapError(err, "Map")
			return
		}
		if za0002 == nil {
			err = en.WriteNil()
			if err != nil {
				return
			}
		} else {
			...
		}
	}
	return
}

// DecodeMsg implements msgp.Decodable
func (z *MyStruct) DecodeMsg(dc *msgp.Reader) (err error) {
	var field []byte
	_ = field
	var zb0001 uint32
	zb0001, err = dc.ReadMapHeader()
	if err != nil {
		err = msgp.WrapError(err)
		return
	}
	for zb0001 > 0 {
		zb0001--
		field, err = dc.ReadMapKeyPtr()
		if err != nil {
			err = msgp.WrapError(err)
			return
		}
		switch msgp.UnsafeString(field) {
		case "Map":
			var zb0002 uint32
			zb0002, err = dc.ReadMapHeader()
			if err != nil {
				err = msgp.WrapError(err, "Map")
				return
			}
			if z.Map == nil {
				z.Map = make(map[string]*Bar, zb0002)
			} else if len(z.Map) > 0 {
				for key := range z.Map {
					delete(z.Map, key)
				}
			}
			for zb0002 > 0 {
				zb0002--
				var za0001 Foo <-----------
				var za0002 *Bar
				za0001, err = StringToFoo(dc.ReadString()) <-----------
				if err != nil {
					err = msgp.WrapError(err, "Map")
					return
				}
				if dc.IsNil() {
					err = dc.ReadNil()
					if err != nil {
						err = msgp.WrapError(err, "Map", za0001)
						return
					}
					za0002 = nil
				} else {
					...
				}
				z.Map[za0001] = za0002
			}
                        ...
}
@klauspost
Copy link
Collaborator

This looks like a quite unobtrusive change. I prefer that to #331 which forces an interface that can be difficult to satisfy for external types.

Ultimately I think "msgp" should allow for all built-in types (except maps) to be keys - perhaps as a commandline opt-in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants