Fix interdiff post-processing with replace chunks overflowing ranges.

Review Request #6030 — Created June 25, 2014 and submitted — Latest diff uploaded

Information

Review Board
release-2.0.x
44201b8...

Reviewers

When post-processing interdiffs in order to filter out unrelated code,
we had issues where replace chunks could be generated where its
orig/modified ranges weren't equal. This would trigger an assert, which
is probably good, because removing the assert resulted in a
corrupted-looking diff.

This same sort of bug could also affect equal chunks in theory, though
in practice, they weren't in as bad a situation.

The reason this was failing was that we attempted to cap each range to
the chunk's boundaries, without any regard to whether or not the lengths
needed to be consistent.

Now, for equal and replace chunks, we get the lengths of the
orig/modified ranges for the chunk, figure out the longest of the two,
cap that within the valid range boundary, and use that as the end
positions for the chunk.

This ensures that we only show the replace/equal range for the section
represented in the union of the two diffs. Anything else outside of that
will turn into a filtered-equal.

I added a new unit test that previously broke but is fixed with this
change. However, I then noticed that another unit test broke. Turns out,
it was already broken. It had resulting replace/equal chunks that didn't
have consistent ranges, and chunks overflowing the bounds. After looking
at the original chunks and throwing away the unit test's bad resulting
chunks, I created a new set based on what they should have been. This
ended up matching the results of the unit test.

I added some new sanity-checking in the unit tests to make sure that any
opcode lists we provide in the future will meet certain criteria, to
help catch such issues in the future.

All unit tests pass, including the fixed version of another test (which has
results computed by hand and not copied/pasted from the results).

Tested with a series of interdiffs I had laying around, and with some broken
interdiffs that were identified.

    Loading...