libserialize: tuple-arity should be provided to Decoder::read_tuple
#17595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @erickt |
danburkert
force-pushed
the
tuple-index-deserialization
branch
from
bb4ca59
to
a95778e
Compare
|
Rebased. |
|
Ping @erickt |
|
Sorry I missed this. Overall this looks good to me. Could you add a test to make sure json does a sensible thing if there are too many or not enough values to deserialize into a tuple? |
danburkert
force-pushed
the
tuple-index-deserialization
branch
from
a95778e
to
4151fc7
Compare
|
Rebased and tests added. |
danburkert
force-pushed
the
tuple-index-deserialization
branch
from
4151fc7
to
4a468d9
Compare
|
Rebased and changed |
|
It's not clear to me whether these test failures are real or an aberration. None of the logs I can see show any failures. |
|
@danburkert way at the bottom: /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libserialize/json.rs:2156: line longer than 100 chars |
Currently `Decoder` implementations are not provided the tuple arity as a parameter to `read_tuple`. This forces all encoder/decoder combos to serialize the arity along with the elements. Tuple-arity is always known statically at the decode site, because it is part of the type of the tuple, so it could instead be provided as an argument to `read_tuple`, as it is to `read_struct`. The upside to this is that serialized tuples could become smaller in encoder/decoder implementations which choose not to serialize type (arity) information. For example, @TyOverby's [binary-encode](https://github.com/TyOverby/binary-encode) format is currently forced to serialize the tuple-arity along with every tuple, despite the information being statically known at the decode site. A downside to this change is that the tuple-arity of serialized tuples can no longer be automatically checked during deserialization. However, for formats which do serialize the tuple-arity, either explicitly (rbml) or implicitly (json), this check can be added to the `read_tuple` method. The signature of `Deserialize::read_tuple` and `Deserialize::read_tuple_struct` are changed, and thus binary backwards-compatibility is broken. This change does *not* force serialization formats to change, and thus does not break decoding values serialized prior to this change. [breaking-change]
danburkert
force-pushed
the
tuple-index-deserialization
branch
from
4a468d9
to
05f6bda
Compare
|
Rebased and line length issue fixed. |
bors
added a commit
that referenced
this pull request
Nov 1, 2014
…=alexcrichton Currently `Decoder` implementations are not provided the tuple arity as a parameter to `read_tuple`. This forces all encoder/decoder combos to serialize the arity along with the elements. Tuple-arity is always known statically at the decode site, because it is part of the type of the tuple, so it could instead be provided as an argument to `read_tuple`, as it is to `read_struct`. The upside to this is that serialized tuples could become smaller in encoder/decoder implementations which choose not to serialize type (arity) information. For example, @TyOverby's [binary-encode](https://github.com/TyOverby/binary-encode) format is currently forced to serialize the tuple-arity along with every tuple, despite the information being statically known at the decode site. A downside to this change is that the tuple-arity of serialized tuples can no longer be automatically checked during deserialization. However, for formats which do serialize the tuple-arity, either explicitly (rbml) or implicitly (json), this check can be added to the `read_tuple` method. The signature of `Deserialize::read_tuple` and `Deserialize::read_tuple_struct` are changed, and thus binary backwards-compatibility is broken. This change does *not* force serialization formats to change, and thus does not break decoding values serialized prior to this change. [breaking-change]
lnicola
pushed a commit
to lnicola/rust
that referenced
this pull request
Aug 29, 2024
Implement lifetime inferring
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently
Decoderimplementations are not provided the tuple arity asa parameter to
read_tuple. This forces all encoder/decoder combos toserialize the arity along with the elements. Tuple-arity is always known
statically at the decode site, because it is part of the type of the
tuple, so it could instead be provided as an argument to
read_tuple,as it is to
read_struct.The upside to this is that serialized tuples could become smaller in
encoder/decoder implementations which choose not to serialize type
(arity) information. For example, @TyOverby's
binary-encode format is
currently forced to serialize the tuple-arity along with every tuple,
despite the information being statically known at the decode site.
A downside to this change is that the tuple-arity of serialized tuples
can no longer be automatically checked during deserialization. However,
for formats which do serialize the tuple-arity, either explicitly (rbml)
or implicitly (json), this check can be added to the
read_tuplemethod.The signature of
Deserialize::read_tupleandDeserialize::read_tuple_structare changed, and thus binarybackwards-compatibility is broken. This change does not force
serialization formats to change, and thus does not break decoding values
serialized prior to this change.
[breaking-change]