Codegen for well-known types

[virtuald]

• Fixes Idiomatic/pointer codegen for well known types (Int64Value, etc) #472

Looks like I found time to fix it... tests seem to pass locally, but I'm sure there are things that could be better. Would love a review.

[virtuald]

The test failure seems to be a side effect of my decision of how to deal with []byte. In particular, both nullable and non-nullable bytes are both []byte. My rationale for this is that there's a difference between []byte(nil) and []byte{}, and that seems like a rather idiomatic way to deal with nullable/non-nullable. However, perhaps that's too subtle, and nullable bytes should be an actual pointer?

@stephen, as the originator of the codegen issue, do you have an opinion on how to deal with nullable/non-nullable bytes?

[virtuald]

I thought about it some this afternoon, and I decided that *[]byte is more obvious and easier to reason about than a []byte that may or may not be a nil value. This also has the side effect of getting rid of a bunch of special cases, so it's a lot simpler. I also just noticed the 'types' testcases, so I added those in too.

Everything looks good finally, would love a review. For your convenience (and mine -- ha!) I split the PR into two commits, one that does the changes and another that contains the regenerated protobufs. Gets rid of a lot of noise.

[donaldgraham]

Thanks so much. @jmarais and I will review this and get back to you...

[donaldgraham]

@virtuald, we're still working through this. We'll let you know when we have some more feedback.

[stephen]

@virtuald thanks for looking into this! we're pretty excited to see this PR.

[virtuald]

ping?

[donaldgraham]

@virtuald, thanks very much for this. @jmarais has gone through it in detail (and it's a lot of code!), and we're delighted to merge it in.

[donaldgraham]

@virtuald, @awalterschulze has suggested another test:

We have a message with the wktpointer option, and one that is exactly the same, but without wktpointer.

Then we make a test that marshals one and unmarshals the other one and vice versa. And check that the marshaled bytes for both are the same.

[virtuald]

Yeah, it's a lot of repetitive code... I actually wrote a python script to help me generate a lot of the conversion code... 😀

I'll take a look next week at adding that test.

[virtuald]

@donaldgraham @awalterschulze isn't that what the tests at https://github.com/gogo/protobuf/tree/master/test/types/combos already does? Sounds like that scenario is already covered. 👍

[awalterschulze]

combos are tests for messages with and without generated marshalers and unmarshalers.
This makes sure that the reflect version of marshaling works with the generated version of unmarshaling, etc.

The test required in this case is more manual.

We want to make sure that a message with annotations marshals to exactly the same bytes as a message without wkt annotations.
Does that make sense?