Re: [PATCH 4/4] line-log: convert an assertion to a full BUG() call

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Aug 7, 2018 at 5:09 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Mon, Aug 6, 2018 at 9:15 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > I think Andrei's assessment is wrong. The code could not test for that
> > earlier, as it did allow ranges to become "abutting" in the process, by
> > failing to merge them. So the invariant you talked about is more of an
> > invariant for the initial state.
>
> My understanding is that range_set_append() is intended to be strict
> by not allowing addition of a range which abuts an existing range
> (although, of course, the assert() checks only the last range, so it's
> not full-proof).

Ignore my parenthetical comment. It is clearly wrong.

Looking at this again, I see that there is some confusion. The comment
in line-log.h says:

    /* New range must begin at or after end of last added range */
   void range_set_append(struct range_set *, long start, long end);

However, that comment was added by me in c0babbe695 (range-set:
publish API for re-use by git-blame -L, 2013-08-06) -- five years and
one day ago -- and it appears to be based upon a direct interpretation
of the condition in the assert(), including its off-by-one error.

*But*, one of the invariants of range-set is that the ranges must
_not_ abut one another, so the "at or after" is wrong; it should say
instead something like "after, and not touching, the end of the last
added range".

That invariant about having a gap between ranges is enforced
deliberately by range_set_check_invariants(). It's also signaled
implicitly by the fact that no callers of range_set_append() ever
invoke sort_and_merge_range_set() after calling range_set_append().