Georg Baum wrote:
> This problem would be easily avoided if one would iterate backwards
> through the macro insets (a macro can never reference another one which
> follows later), but unfortunately I failed to implement that, since I do
> not understand the DocIterator stuff well enough. I tried several
> versions, both using nextInset() and prevInset() in
> DocIterator::backwardInset() and Buffer::updateMacroInstances(), but
> nothing worked. The attached patcdh shows one failed attempt. Does anybody
> have a clue how to implement a backward inset iteration?
After further investigation it turned out that backward iteration did not
fix the problem. A nice side effect of that investigation is that we now
have a working DocIterator::backwardInset().
The real cause for the bug is that ArgumentProxy::mathMacro_ is a reference
to an object which is stored in a MathData, which is a std::vector storing
MathAtoms by value (not pointers). Therefore, each time when the MathData
object which contains a math macro instance is resized, the
ArgumentProxy::mathMacro_ members of its arguments may become invalid. In
case of bug 9418 the resizing happens because only macro \a is copied to the
clipboard, and macro \b is not, therefore \b is converted to an unknown
inset and its argument is put as a separate inset after \b.
ArgumentProxy::mathMacro_ is broken by design IMHO, but fixing this properly
will involve rewriting big parts of the math macros (which would probably be
a good thing, but I don't have the time for it). Therefore I propose the
attached ugly patch, which ensures for the case that a MathData object is
resized by MathData::updateMacros() that the references are valid. This
fixes the crash in bug 9418, it may also fix other crashes, but similar
crashes might still exist if a MathData object is resized without an
immediately following updateMacros().
Does anybody have a better idea?
Georg
diff --git a/src/mathed/MathData.cpp b/src/mathed/MathData.cpp
index 1b79f0c..48e4ffa 100644
--- a/src/mathed/MathData.cpp
+++ b/src/mathed/MathData.cpp
@@ -394,7 +394,7 @@ void MathData::updateBuffer(ParIterator const & it, UpdateType utype)
void MathData::updateMacros(Cursor * cur, MacroContext const & mc,
- UpdateType utype)
+ UpdateType const utype)
{
// If we are editing a macro, we cannot update it immediately,
// otherwise wrong undo steps will be recorded (bug 6208).
@@ -403,6 +403,8 @@ void MathData::updateMacros(Cursor * cur, MacroContext const & mc,
docstring const edited_name = inmacro ? inmacro->name() : docstring();
// go over the array and look for macros
+ size_t const oldsize = size();
+ vector<size_t> macroindices;
for (size_t i = 0; i < size(); ++i) {
MathMacro * macroInset = operator[](i).nucleus()->asMacro();
if (!macroInset || macroInset->name_.empty()
@@ -492,6 +494,24 @@ void MathData::updateMacros(Cursor * cur, MacroContext const & mc,
inset = inset->asScriptInset()->nuc()[0].nucleus();
LASSERT(inset->asMacro(), continue);
inset->asMacro()->updateRepresentation(cur, mc, utype);
+ macroindices.push_back(i);
+ }
+
+ if (size() == oldsize)
+ return;
+
+ // If our size has changed because macro parameters have been detached
+ // or attached, we need to update the representation of all macros
+ // again, since the references stored in ArgumentProxy may now be
+ // invalid (bug 9418). This will not do any visible change except
+ // creating ArgumentProxy instances with valid references.
+ for (size_t i = 0; i < macroindices.size(); ++i) {
+ InsetMath * inset = operator[](macroindices[i]).nucleus();
+ if (inset->asScriptInset())
+ inset = inset->asScriptInset()->nuc()[0].nucleus();
+ MathMacro * macroInset = inset->asMacro();
+ LASSERT(macroInset, continue);
+ macroInset->updateRepresentation(cur, mc, utype);
}
}
