On 2018/07/10 18:09:19, dak wrote:
On 2018/07/10 17:49:33, dak wrote: > On 2018/07/10 17:10:53, benko.pal wrote: > > LGTM; just by looking I can't see how it can make make fail. > > using rp[-2] and rp[-1] instead of lastcol and c would be cleaner
to me, but
> > YMMV. > > Since the whole point is to do the operation in-place, rp[-2] may
already have
> been overwritten by rp[-1] in the last iteration. It may seem
cleaner to you
to > use rp[-2] but it would be a rather ugly bug. Doing things in-place
is more
> efficient but you have to keep track of what you are doing.
ah yes, thanks for pointing this out! actually this may worth some comments; I mean, documenting why looseness decision is based only on original neighbours, not on after-pruning neighbours (I know rp[-2] may not be the after-pruning neighbour either). this may be obvious, but I feel uneasy by just reading this file.
> I consider it possible that mixing a const iterator with a non-const
one might
> be what caused the compilation error but I'd want to see the error
message to
> be sure.
Just seen <https://stackoverflow.com/a/12662703> which indicates that
as of
C++11, you can use a const_iterator for a call to erase (since the
modifiability
is ensured by the object erase is called on and the const-ness of the
iterator
is irrelevant) but apparently not previously. So I guess it is my
newer
compiler that made the erase call work without complaint. I'll see
how I can
make this C++08(?)-save.
that was my guess too, but while I messed with git and make, you beat me :) looks still good to me. https://codereview.appspot.com/351720043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel