Avoid encoding 0 values for proto3 fields

[mmsqe]

current behaviour:
when a enum field is set with zero-value, the value is encoded in the buffer

expected behaviour:
when a enum field is set with zero-value, the value is omitted in encoding

[sigurdm]

Is this relating to the binary encoding?
What does it mean to set it to zero-value? Are you using the 'clearX' method
Could you provide a small reproduction sample?

[mmsqe]

@sigmundch Not sure if it's protobuf standard since I need to add it mannualy after generating transaction.proto like below:

While Libra use rust-protobuf 2.7.0 will skip 0, no byte at all

if self.sequence_number != 0 {
    my_size += ::protobuf::rt::value_size(2, self.sequence_number, ::protobuf::wire_format::WireTypeVarint);
}

[sigurdm]

Ah - I think I get it now.
I don't think this is about enum, but integer fields.
They should not be represented if they have the non-default value.

We could do that either when encoding or in the setter...
First we need to store in the metadata for each field has proto3 semantics.

Is this issue blocking you, or is it a nice-to-have?

[mmsqe]

@sigmundch thanks for the explanation, no rush here

[calmh]

This is a somewhat blocking issue for me, for the following reason... Consider a trivial message with just an integer (or enum) field.

message Foo {
   int field = 1;
}

We can instantiate this with a zero field either by Foo() or Foo()..field = 0. These two don't compare equally using ==, which is also unexpected. But more importantly, an incoming message over the wire won't have the zero value present, meaning if we construct a Foo()..field = 0 and compare it to the corresponding message deserialized from the wire it won't be equal. If we do the same with any other integer value the are equal...

I would expect the default value handling to mean that messages are considered equal regardless of whether an int value is unset or set to zero.

Perhaps this is somewhat different than the issue subject here, but fixing this issue (not serializing zero fields) would highlight the problem further by making an object with a zero field not survive an encoding roundtrip and still compare equally. Currently it does, but only because zero fields are sent as explicit zeroes.

[osa1]

This is a somewhat blocking issue for me, for the following reason... Consider a trivial message with just an integer (or enum) field.

message Foo {
int field = 1;
}

We can instantiate this with a zero field either by Foo() or Foo()..field = 0. These two don't compare equally using ==, which is also unexpected. But more importantly, an incoming message over the wire won't have the zero value present, meaning if we construct a Foo()..field = 0 and compare it to the corresponding message deserialized from the wire it won't be equal. If we do the same with any other integer value the are equal...

I would expect the default value handling to mean that messages are considered equal regardless of whether an int value is unset or set to zero.

Perhaps this is somewhat different than the issue subject here, but fixing this issue (not serializing zero fields) would highlight the problem further by making an object with a zero field not survive an encoding roundtrip and still compare equally. Currently it does, but only because zero fields are sent as explicit zeroes.

This is indeed a different issue. The issue in your case is this part:

These two don't compare equally using ==, which is also unexpected

I thought we had an issue for this already but I can't find it now. We should open an issue for this if we don't have one already.

As far as I can see proto specs do not say that default values for optional fields must be omitted. Only proto3 JSON spec says that (see #592, #585). So labeling this is "perf" instead of "bug".

[calmh]

The proto3 language guide says about default values:

Also note that if a scalar message field is set to its default, the value will not be serialized on the wire.

It doesn't use the word "must" in capital letters or so, but I think it's pretty well established that this is how proto3 should work...

[osa1]

Thanks for pointing that out, I had missed that. Marking this issue as bug.