On 08/09/2009 03:00 PM, Vincent van Ravesteijn wrote:
Abdelrazak Younes schreef:
On 09/08/2009 20:48, Vincent van Ravesteijn wrote:
Abdelrazak Younes schreef:
Hi,
There used to be a recursive call to setBuffer() at the top of
Buffer::updateLabels(); is there a reason why this has been
removed? Now we get crashes with a lot of LFUNs because of the
buffer absence. Try LFUN_PARAGRAPH_MOVE_UP for example...
Abdel.
The reason is that we decided that it shouldn't be there.
And the reason why you decided this?
:-)
Abdel.
To be slightly more helpful.
There was a large FIXME next to this call that this should be done in
the pasteSelectionHelper, so we moved it there:
// FIXME Should we do it here, or should we let updateLabels() do it?
// Set paragraph buffers. It's important to do this right away
// before something calls Inset::buffer() and causes a crash.
for (pit_type p = startpit; p <= pit; ++p)
pars[p].setBuffer(const_cast<Buffer &>(buffer));
We came to this because of some bug which apparently wanted to have
access to the buffer before updateLabels was called, I don't know the
details anymore.
Pasting branches, or something like that.
Afterwards we fixed some of those missing buffer problems, like in
outlineUp and friends.
Basically we think it's a bit lazy to start moving around paragraphs
without ensuring that a decent buffer is set. Moreover, I think that
we should do it only at one place. That place would be somewhere in
CutAndPaste. There is no other reason than pasting that the buffer
should get invalidated.
I think I actually did the removal, and it was for the reasons Vincent
mentions. We did have to fix some assertions, but overall it seemed as
if they needed fixing anyway. Whenever we do something that might
invalidate the buffer_ members, we have to deal with it ASAP, because
you never know when something might try to access that member. (That was
the old bug.)
So I do think Vincent is right about this, in principle.
I'm not sure why swap(pars_[pit], pars_[pit-1]) would invalidate the
buffer ?
I'm also puzzled about this. Isn't swap NOT supposed to make copies but
just to move pointers around?
Richard
