Add VarZeroVecFormat support to VarTuple and make_var #5808
Conversation
it's internal: "user convenience" is irrelevant and this is safer
Manishearth
force-pushed
the
makevar-format
branch
2 times, most recently
from
f978e56 to
d64044e
Compare
| assert!(!fields.is_empty(), "Must have at least one unsized field"); | ||
| Self { fields } | ||
|
|
||
| let format_param = format_param.unwrap_or_else(|| quote!(zerovec::vecs::Index16)); |
There was a problem hiding this comment.
Thought: make it a required parameter?
There was a problem hiding this comment.
I'd rather not: I want the user-facing things (VarZeroVec, make_varule, TupleNVarULE) to default to Index16. Formats are a power user tool IMO.
| fn test_tripleule_validate() { | ||
| test_tripleule_validate_inner::<Index8>(); | ||
| test_tripleule_validate_inner::<Index16>(); | ||
| test_tripleule_validate_inner::<Index32>(); |
There was a problem hiding this comment.
Suggestion: add a test that serializing and deserializing between different formats is usually an error (unless we're unlucky and the bits are valid in both formats)
There was a problem hiding this comment.
I don't see the point of a test like that. If it succeeds, it's a valid representation. We already have scattered tests that ensure that invalid types don't parse with carefully chosen non-parsing data, I don't see any benefit to writing one around swapping formats specifically.
Manishearth
force-pushed
the
makevar-format
branch
from
d64044e to
d7c8d0f
Compare
Manishearth
force-pushed
the
makevar-format
branch
from
ff2cc52 to
1f96ed4
Compare
Part of holistic work in #5523
Part of #2312 (Somehow never filed a separate issue for this, but I mentioned it in a commetn)