Let the shim functions return an error

[zond]

At least the decode implementation usually has a few error cases, and it would be nice if those errors could be properly propagated.

[philhofer]

Agreed

[zond]

Do you want me to start hammering out a pull request, or will you do this? :)

[philhofer]

I probably won't have time to look at this until later this week. Pull requests more than welcome.

[zond]

I will try to take a look at it the next few days.

Any pointers on where to look?

[philhofer]

The rabbit hole starts here.

You'll also want to look at gen.BaseElem and its respective printing functions in gen/encode.go, gen/decode.go, gen/marshal.go, etc.

[philhofer]

It may make sense to implement this as a different directive. It may be tough to preserve backwards-compatibility otherwise.

[zond]

I just ran out of time after fixing a bug in one of my projects, and then trying out your appengine build tag stuff. I will try to look at this during the weekend.

[shabbyrobe]

@philhofer, would you be happy for me to pick this up? I really need this functionality too and I'm happy to do the work. Agree it should be a different directive.

[philhofer]

Yeah, if you feel like taking a shot at this, go for it.

…

On Jul 4, 2017, at 10:22 PM, Blake Williams ***@***.***> wrote: @philhofer, would you be happy for me to pick this up? I really need this functionality too and I'm happy to do the work. Agree it should be a different directive. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[shabbyrobe]

OK I have a first pass of this, some feedback would be great! Ended up adding an optional extra argument to the directive instead of a new directive. The zero value defaults to the existing behaviour:

//existing behaviour is default:
//msgp:shim MyType as:string using:fromMyType/toMyType

//explicit declaration of existing behaviour:
//msgp:shim MyType as:string using:fromMyType/toMyType mode:cast

//new mode:
//msgp:shim MyType as:string using:fromMyType/toMyType mode:convert

I ran into some sticky stuff with the size calculations. These shim functions could be doing all kinds of different stuff under the hood so it seems like it may be a performance tradeoff between running the shim function twice to get an accurate size or running it once and guessing the size, even if that guess will almost certainly be wrong in the case where the base type is a string. Either that or I'm missing something about how to do it more effectively... there's a good chance of that.

"Convert" might be a less than ideal name for it too as there is already a member on BaseElem called Convert, but I couldn't think of something better at the time.

[shabbyrobe]

Reckon we can close this one off or does it still need a bit more work?