[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Spacing_spanner::prune_loose_columns: prune in-place (issue 35172004
From: |
dak |
Subject: |
Re: Spacing_spanner::prune_loose_columns: prune in-place (issue 351720043 by address@hidden) |
Date: |
Tue, 10 Jul 2018 11:09:19 -0700 |
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.
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.
https://codereview.appspot.com/351720043/