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

Add generator option to always produce pointer receivers #332

Closed
mjmar01 opened this issue Oct 19, 2023 · 3 comments · Fixed by #357
Closed

Add generator option to always produce pointer receivers #332

mjmar01 opened this issue Oct 19, 2023 · 3 comments · Fixed by #357

Comments

@mjmar01
Copy link

mjmar01 commented Oct 19, 2023

I'd like to propose a command line parameter for the generator like "receiver-preference" or similar to modify the receiver logic for Marshal and MsgSize methods. So it could either do:

  • Pointer receivers for everything
  • Pointer receivers for unmarshal methods, but non-pointer receivers for marshal methods (this might also satisfy a previous issue)
  • Dynamic like before

Currently the generator will frequently mix receiver types for generated methods. This causes annoyances in some scenarios like for example when using both the Unmarshaler and Marshaler interfaces in another custom interface. Since generated types often don't implement both of these interfaces at once, the custom interface might also become unusable.

Currently you could work around this by including unexported and omitted fields in the struct to fool the generator into using pointer receivers:

//go:generate msgp -unexported

type MyCustomType struct {
	forcePtrRcvr     struct{}    `msg:"_,omitempty"`
	ArbitraryField   int
}

But this is unclean and doesn't work when encoding to tuples using //msgp:tuple MyCustomType, when using basic types (not structs) or when unexported fields should not be serialized at all.

Let me know what you think and if I can provide further information or assistance :)

@klauspost
Copy link
Collaborator

If implemented as directives, I for sure wouldn't mind it. Keeping the commandline "clean" is IMO better design.

To me the cleanest would be:

//msgp:pointer

... simply as a file level directive.

@klauspost
Copy link
Collaborator

type NamedBool    bool

Will need some special care, as forcing pointers on these means the code will also need to change.

Also we maybe have to deal with the fact that the receiver can now be nil.

@klauspost
Copy link
Collaborator

See #357

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

Successfully merging a pull request may close this issue.

2 participants