[lmi] How InputSequenceEditor::remove_row() works [Was: master 78a4de0 5/7: Enable '-Wnull-dereference'; fix the issues it flags]

[Greg Chicares]

On 2019-03-22 15:41, Vadim Zeitlin wrote:
> On Fri, 22 Mar 2019 12:49:37 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> But if you want to write a comment explaining this, I'll gladly
> GC> commit it.
> 
>  Here is my attempt:
> 
>       // Note that the index here is constant and always refers to
>       // the first window in the given row: as the indices of the
>       // subsequent elements adjust, by decreasing by one, when we
>       // delete this index, repeatedly deleting Col_Max elements at
>       // this position results in deleting the entire row contents.
> 
>  Does this help?

Yes, thanks. Committed; pushing as soon as all tests complete.

Here's how I understand it now. These enumerators:

enum Col 
{Col_Value,Col_From,Col_DurationMode,Col_DurationNum,Col_Then,Col_Remove,Col_Add,Col_Max};
values:   0         1        2                3               4        5        
  6       7

correspond to this example:

        Col_Value       Col_From   Col_DurationMode 
Col_DurationNum,Col_Then,Col_Remove,Col_Add,Col_Max
    //  Employee payment:
    //    [   0]  from issue date until [year]      [ 5]            , then   
[REMOVE]   [ADD]   [nothing]
    //    [1000]  from   year 5   until [year]      [10]            , then   
[REMOVE]   [ADD]   [nothing]
    //    [   0]  from   year 10  until [ age]      [70]            , then   
[REMOVE]   [ADD]   [nothing]
    //    [   0]  from   age 70   until [maturity]                  .

So we have an Nx7 array (4x7 in the example above), whose index-origin-zero 
indices are
   0  1  2  3  4  5  6  7
   8  9 10 11 12 13 14 15
  16 17 18 19 20 21 22 23
  24 25 26 27 28 29 30 31

Suppose we want to erase the third row (row 2 in origin zero), whose indices are
  16 17 18 19 20 21 22 23
We multiply
  2, which is the index of the third row, by
  7, which is Col_Max
obtaining 14 as our offset. Then, starting from offset 14, we erase seven
(i.e., Max_Col) elements.

Huh?

It looks to me like there's an off-by-one error there, and we should really
start erasing at offset 16 = (Max_Col +1) * 2. In that case, the code would
probably do something that looks right, anyway...removing the [ADD] and
[nothing] elements of the second row as well as the first six elements of
the third:

   0  1  2  3  4  5  6  7
   8  9 10 11 12 13 XX XX
  XX XX XX XX XX XX 22 23
  24 25 26 27 28 29 30 31

and the result would probably be okay.

Or maybe I've misunderstood Max_Col, and it's really not an "element",
but a "one-past-the-end" sentinel of measure zero...in which case we'd have

   0  1  2  3  4  5  6
   7  8  9 10 11 12 13
  14 15 16 17 18 19 20
  21 22 23 24 25 26 27

and the offset of 14 calculated above would be correct.

I'd point out how trivial this would be in APL, but I promised myself I'd
hit 'Send' by the time the complete test battery completed, so...sending.