Feedback Wanted: Explicitly type byte vectors using 0u8

[teor2345]

In #809 (comment), @dconnolly suggested we use [0u8; N] for byte arrays, rather than [0; N].

This change makes sure that the array is a byte array, rather than risking type inference choosing another integer size. (Or defaulting to i32.)

However, it comes with a readability cost, so I'm looking for feedback before we merge.

This is an automatic search and replace, using this script:
sed -i -e "s/[0;/[0u8;/g" find zebra* -name '*.rs'

[yaahc]

I think its relatively unlikely that we'd ever create an array type that needs to be an array of bytes to avoid a bug and then never use that array in a position that lets the compiler infer the correct type. So I'm not particularly excited about this change. If everyone else wants this change I'm not opposed, I don't think it hurts anything, even readability is fine IMO, but likewise I don't think it helps much either.

[dconnolly]

I think its relatively unlikely that we'd ever create an array type that needs to be an array of bytes to avoid a bug and then never use that array in a position that lets the compiler infer the correct type. So I'm not particularly excited about this change. If everyone else wants this change I'm not opposed, I don't think it hurts anything, even readability is fine IMO, but likewise I don't think it helps much either.

i'm having a major deja vú right now 😵

[dconnolly]

I accidentally'd the button 🙈

[dconnolly]

I don't have a very strong opinion on this, so I'm open to consensus

[daira]

I very much like this explicitness.

[yaahc]

As a non strong opinion haver here I'm happy to merge this to support those who do have strong opinions.

[hdevalence]

My feeling is basically the same as @yaahc's. I'm weakly against making this change, because I think that type annotations shouldn't be used unless they're needed. Otherwise there's no way to distinguish type annotations that are meaningful from type annotations that just replicate an already-inferred type constraint.

I think its relatively unlikely that we'd ever create an array type that needs to be an array of bytes to avoid a bug and then never use that array in a position that lets the compiler infer the correct type.

I'd go further and say that the fact that there's no type constraint on the array means that it doesn't need to be an array of bytes.

[dconnolly]

Basically my intuition here is, be explicit when needed, but not by default. So I wouldn't apply this change. Maybe an explicit let thing: [u8; 32] = [0; 32]; would be fine when needed?

[teor2345]

I find let thing: [u8; 32] = [0; 32]; easier to read than let thing = [0u8; 32];.

In the 0u8 case, I have to distinguish between numbers that are values, and numbers that are sizes.

Would anyone like the opposite change?
(That is, replacing 0u8 with 0, when the size is not required.)

[yaahc]

Would anyone like the opposite change?
(That is, replacing 0u8 with 0, when the size is not required.)

I would favor the opposite change, but once again, not a strong opinion haver.

[daira]

It may be worth pointing out that zcashd/librustzcash code uses [0u8; ...] for byte arrays: https://github.com/zcash/zcash/search?q=0u8&unscoped_q=0u8 and https://github.com/zcash/librustzcash/pull/258/files for example.

I don't see why as a general principle it would be undesirable to explicitly type a value whose type would have been inferred (the idea that you want the code to be maximally general doesn't really apply in this case: you rarely if ever want byte arrays to be more general, because that's significantly less efficient). I also don't see why the more verbose let thing: [u8; 32] = [0; 32]; is preferable to let thing = [0u8; 32];, which says the same thing to me, and seems like a very natural idiom for a byte array.

[hdevalence]

I don't see why as a general principle it would be undesirable to explicitly type a value whose type would have been inferred

The problem is that when you do this, it's harder to distinguish which type annotations have intent and which don't, i.e., which are intended to be meaningful and which are implied from other constraints. Like comments, type annotations should be used when there is some special intent to be conveyed to the reader, not just to describe what the code is doing.

[daira]

It's always meaningful that a byte array is a byte array, right? It's much less common to use byte arrays just because you need an array of integers that happen to be in the range 0..255, than to use a byte array because it's an I/O representation. (There are no cases of the former in this PR.)