unusual_byte_groupings fires for octal values represented in binary

[koutheir]

Lint name: unusual_byte_groupings

I tried this code:

let unix_mode = 0b111_101_101; // rwx r-x r-x

I didn't expect to see the warning digits of hex or binary literal not grouped by four, because grouping by 3 bits is very reasonable when binary is clearer than octal.

Meta

• cargo clippy -V:

  clippy 0.0.212 (e1884a8 2020-12-29)
  

• rustc -Vv:

  rustc 1.49.0 (e1884a8e3 2020-12-29)
  binary: rustc
  commit-hash: e1884a8e3c3e813aada8254edfa120e85bf5ffca
  commit-date: 2020-12-29
  host: x86_64-unknown-linux-gnu
  release: 1.49.0
  

[camsteffen]

Hmm I guess we'll just add a special case for 3 groups of 3? Usually the groups don't have semantic meaning like they do in a unix mode.

[koutheir]

I think this doesn't depend on any semantic meaning. Grouping by four bits is a specification of one hexadecimal digit in the binary numeral system. Likewise, grouping by three bits is a specification of one octal digit in binary. The octal numeral system is used often enough to make this warning unexpected in computer science.

[mohanson]

In virtual machine development, we often need to encode/decode an instruction, and the instruction encoding usually has its own format, such as RISC-V ISA:

// |31|30|29|28|27|26|25|24|23|22|21|20|19|18|17|16|15|14|13|12|11|10|09|08|07|06|05|04|03|02|01|00|
// |funct7              |rs2           |rs1           |funct3  |rd            |opcode              | R-type
// |rs3           |funct|rs2           |rs1           |funct3  |rd            |opcode              | R4-type
// |imm[11:0]                          |rs1           |funct3  |rd            |opcode              | I-type
// |imm[11:5]           |rs2           |rs1           |funct3  |imm[4:0]      |opcode              | S-type
// |imm[31:12]                                                 |rd            |opcode              | U-type
// |12|imm[10:5]        |rs2           |rs1           |funct3  |imm[4:1]   |11|opcode              | B-type
// |20|imm[10:1]                    |11|imm[19:12]             |rd            |opcode              | J-type

I think the addition of this check is an impulsive decision, and it should never exist, whether it is a 4-bit grouping or a 3-bit grouping, when people choose to use binary to represent a number, they understand what they are doing better than Clippy.

[camsteffen]

I think this doesn't depend on any semantic meaning. Grouping by four bits is a specification of one hexadecimal digit in the binary numeral system. Likewise, grouping by three bits is a specification of one octal digit in binary. The octal numeral system is used often enough to make this warning unexpected in computer science.

Good point. Seems very sensible to not lint groups of three.

In virtual machine development,

Certainly there can be any grouping for any proprietary format. But these are "unusual". However, maybe they really are not that unusual and so this lint should not warn by default?

[koutheir]

Certainly there can be any grouping for any proprietary format. But these are "unusual". However, maybe they really are not that unusual and so this lint should not warn by default?

I think formats that deal with bit fields are uncommon enough to keep this lint enabled by default.

[nindalf]

@rustbot claim

[llogiq]

Fixed by #10353.