Use a Config trait to configure bincodes compile-time decisions

[TyOverby]

This design will not only reduce the complexity of many type signatures, but it'll also allow us to add more features without breaking back-compat.

This is because the config trait is not implementable by customers of the library (it has a trait bound of a hidden type), and the BasicConfig object is not referenceable by type name because it is parameterized by a hidden type.

What this means practically is that adding another option to bincode like "use variable length encoding" is possible to add as an option without it being a breaking change.

[dtolnay]

Neat idea. What are the advantages of including SizeLimit in this config and handling it with Bincode-specific code throughout?

On the deserialization side, it seems like size limit is configuring a property of the IO stream not a property of Bincode, so to me it doesn't belong in the Bincode config. Implementing this as a wrapper that puts a cap on the number of bytes read from a reader would make it reusable across all formats. Also it could potentially be implemented in a significantly more performant way for buffered readers. We only need to check the size limit once per filling of the buffer, not every single time a piece is read from the buffer.

On the serialization side, many of the uses of Bounded I saw on GitHub were pretty worthless. For example this one.

packet.append(&mut serialize::<_,_,NetworkEndian>(&(p as u16), Bounded(2)).unwrap());

I would prefer to push people to use serialized_size_bounded because it makes it clear at the call site what state the stream is left in if the input is too big.

[dtolnay]

The code says LittleEndian.

[TyOverby]

Fixed

[dtolnay]

Also I would like an API that is more friendly to the overwhelmingly common case than this.

bincode::serialize(&t, DEFAULT_CONFIG)

Maybe something like:

bincode::serialize(&t)
bincode::little_endian().serialize(&t)
bincode::bounded(50<<20).little_endian().serialize(&t)

[TyOverby]

What are the advantages of including SizeLimit in this config

Benefit is that all configuration trait parameters go in one trait.

and handling it with Bincode-specific code throughout?

I firmly believe that size-limit should be a part of the bincode API, though maybe not as prominently (or as baked-in) as it currently is. I'd be willing to do some experiments with making this generic over Readers.

Also I would like an API that is more friendly to the overwhelmingly common case than this.

Your examples look great! I think that something like this would be perfect for the final API.

[dtolnay]

Seems good to me as long as we agree to change the serialize(&t, DEFAULT_CONFIG) API before release this.

Also be careful because this code would be broken by adding another type parameter:

struct S<E: ByteOrder, L: SizeLimit, O> {
    bincode: BasicConfig<E, L, O>
}

[TyOverby]

Also be careful because this code would be broken by adding another type parameter:

Damn, I was really hoping to avoid this. Is there any way to keep people from storing references to such structs?

[dtolnay]

Yes but I think keeping people from storing references to such structs is not the best approach. How about defining BasicConfig without any type parameters? Then when serializing / deserializing, as early as possible turn it into a private generic one similar to BasicConfig in this PR.

[TyOverby]

Interesting thought. Yeah, if we use the builder API like you've proposed above, it wouldn't be necessary to have the generic type parameters.

[TyOverby]

Closed in favor of #217