Skip to content

Rewrite the hex accumulation so no mutation is equivalent #2209

Description

@gaborbernat

Problem

stream/transformer.rs:411 accumulates a \uXXXX escape by bit-packing:

let value = value << 4 | u16::from(digit);

The |^ mutation of this line cannot be observed. value << 4 leaves the low four bits zero and digit is a hex nibble in 0..=15, so the operands never share a set bit and OR and XOR agree on every reachable input.

That is a property of the spelling, not of the computation. The | is doing the work of an addition over disjoint bit ranges, and any of |, ^ or + would compute the same thing — which is precisely why mutating between them is invisible. An operator whose choice cannot be observed is an operator whose choice was arbitrary.

Required change

Rewrite the accumulation so every operator carries meaning:

let value = value * 16 + u16::from(digit);

This is the same computation for disjoint operands, and both of its mutations are observable: */ and +- each change the result, so a test over \uXXXX decoding kills them. The |/^ ambiguity disappears along with the |.

Check the rewrite is faithful before taking it. A \uXXXX escape carries at most four hex digits, so the accumulated value stays within u16 and the multiply cannot overflow where the shift would have wrapped. Confirm that bound rather than assuming it, and confirm the surrounding loop cannot feed a fifth digit.

Acceptance criteria

  • decode_key_byte has no mutation that survives a test.
  • The new operators are killed by a test over \uXXXX key decoding, each verified by counterfactual.
  • No .cargo/mutants.toml exclusion is added for this line.
  • Behaviour is unchanged for every four-digit escape, including the maximum .

Boundary

Only the accumulation expression and the tests that cover it.

History

Filed first as an exclusion request, on the reasoning that an equivalent mutant cannot be killed by any test. That is true and beside the point: the mutant is only equivalent because of how the line is written, and rewriting it removes the equivalence. The same revision wrongly claimed emit_file's two > mutants at transformer.rs:661 needed a page of 500,000 files — files_seen is a field, so a test seeds it to the boundary and calls emit_file once. Those are ordinary #1893 work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:ciContinuous integration and repository checksarea:testsConformance, perf tests, and observabilitypriority:P2Performance, observability, or deferred featuretype:bugIncorrect behavior or missing validationtype:testTesting, conformance, metrics, or validation coverage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions