Add proto3 specific deserialization that requires fewer SRTP constraints

[jhugard]

Going this route, the proto2 specific code should probably go in a Proto2 module. Breaking change, yes, but cleaner.

Also, would it be possible to define common functions in terms of the other module to eliminate duplication? E.g., in module Proto2, define let inline fromZeroCopyBuffer m zcb = Proto3.fromZeroCopyBuffer m zcb? As in this example, I'd make Proto3 the base case, and define Proto2 in terms of Proto3.

[7sharp9]

I did think about sharing the core stuff but most of the functions share just a shard:

    module Shared =
        /// Deserialize a message from a ZeroCopyBuffer, given a default message.
        let inline fromZeroCopyBuffer f m zcb =
            zcb
            |> Utility.decodeBuffer
            |> f m
            |> Helpers.Proto2.decodeFixup

        /// Deserialize a length-delimited message from a ZeroCopyBuffer, given a default message.
        let inline fromZeroCopyBufferLengthDelimited f m zcb =
            zcb
            |> Utility.unpackLengthDelimited
            |> f m
            |> Helpers.Proto2.decodeFixup

        /// Deserialize a message from a length-delimited RawField, given a default message.
        let inline fromRawField f m (rawField:RawField) =
            let buf = 
                match rawField with
                | LengthDelimited (fieldId, buf) ->
                    buf
                | _ ->
                    raise <| SerializerException(sprintf "Expected LengthDelimited field, found %s" (rawField.GetType().Name) )
            buf
            |> ZeroCopyBuffer
            |> fromZeroCopyBuffer f m

    module Proto2 =
        /// Deserialize a message from a ZeroCopyBuffer, given a default message.
        let inline fromZeroCopyBuffer m zcb =
            Shared.fromZeroCopyBuffer Helpers.Proto2.deserializeFields m zcb

        /// Deserialize a message from a ZeroCopyBuffer, given a default message.
        /// Shorthand for Deserialize.fromZeroCopyBuffer.
        let inline fromZcb m zcb = fromZeroCopyBuffer m zcb

        /// Deserialize a length-delimited message from a ZeroCopyBuffer, given a default message.
        let inline fromZeroCopyBufferLengthDelimited m zcb =
            Shared.fromZeroCopyBufferLengthDelimited Helpers.Proto2.deserializeFields m zcb

        /// Deserialize a length-delimited message from a ZeroCopyBuffer, given a default message.
        /// Shorthand for Deserialize.fromZeroCopyBufferLengthDelimited.
        let inline fromZcbLD m zcb = fromZeroCopyBufferLengthDelimited m zcb

        /// Deserialize a message from a length-delimited RawField, given a default message.
        let inline fromRawField m (rawField:RawField) =
            Shared.fromRawField Helpers.Proto2.deserializeFields m rawField

        /// Deserialize a message from a length-delimited RawField, given a default message,
        /// and return Some(message).  Used to simplify the call-site when deserializing
        /// inner messages.
        let inline optionalMessage m rawField =
            Some (fromRawField m rawField)

        /// Deserialize a message from an ArraySegment, given a default message.
        let inline fromArraySegment m (buf:ArraySegment<byte>) =
            buf
            |> ZeroCopyBuffer
            |> fromZcb m

        /// Deserialize a length-delimited message from an ArraySegment, given a default message.
        let inline fromArraySegmentLengthDelimited m (buf:ArraySegment<byte>) =
            buf
            |> ZeroCopyBuffer
            |> fromZcbLD m

        /// Deserialize a length-delimited message from an ArraySegment, given a default message.
        /// Shorthand for Deserialize.fromArraySegmentLengthDelimited.
        let inline fromArraySegmentLD m buf = fromArraySegmentLengthDelimited m buf

        /// Deserialize a message from a byte array, given a default message.
        let inline fromArray m buf =
            buf
            |> ArraySegment
            |> fromArraySegment m

        /// Deserialize a length-delimited message from a byte array, given a default message.
        let inline fromArrayLengthDelimited m buf =
            buf
            |> ArraySegment
            |> fromArraySegmentLD m

        /// Deserialize a length-delimited message from a byte array, given a default message.
        /// Shorthand for Deserialize.fromArrayLengthDelimited.
        let inline fromArrayLD m buf = fromArrayLengthDelimited m buf

Code like that starts to have quite bad readability for the code sharing it offers

[7sharp9]

Ive updated the PR to have as much sharing as possible. I have also placed proto2/3 in separate modules.

Deserialization now has to be explicit. As the scope of the deserialization is code generation this is probably a good thing in light of possible proto2/3 confusion, all it takes is opening the required module to access what you need.

[jhugard]

Other than the above comments, the changes look very clean. Thank you!

[7sharp9]

I have nothing against the optional support but having the fields in the record definition seems very hacky thats all.

I think the way forward would be still to have the separate proto3 deserializer as theres still some proto3 functions that are not needed I think.

[jhugard]

I have nothing against the optional support but having the fields in the record definition seems very hacky thats all.

I couldn't figure a way to have those fields in each record, but not need to provide them when instantiating the record. Did you have an idea here that would be cleaner?

[7sharp9]

The simplest thing is to check the decoderRing and use field 0 like proto2 except have the behavior of not throwing is there no decoder. Then its up to the code gen to add the zero field for unknowns which should be trivial. the generator could be given parameters whether to create an unknown field section in the record definition.

[7sharp9]

Fundamentally record construction makes the unknowns tricky, the only thing I could think of was generating a static construction method with the last param being optional. You would have to forfeit the record construction syntax to use the static method though.

[7sharp9]

With a class thats generated it would be easy to just have a ResizeArray etc and a method to get unknown fields, but theres no mechanism available with a record.

[7sharp9]

@jhugard I added a few tests as requested.

[jhugard]

Better, but suggest a touch of cleanup to show your desired Proto3 usage.

[7sharp9]

OK, I have now added some details in the proto3 test types.

[jhugard]

Great work. Thank you!