Jean-Marc Lasgouttes wrote:
> Le 20/03/15 21:53, Georg Baum a écrit :
>> 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.
>
> What about using a std::list or our own RandomAccessList instead of
> std::vector?
Good idea. The random access is needed, so std::list does not work. Using
RandomAccessList seems to work, but MathData is a very central part of
mathed, so this would need very intensive testing. Also, some of the loops
should be rewritten using iterators to avoid recomputation of the current
iterator. General performance testing (memory and speed) would be needed as
well.
In total I am not sure whether we should go that way. I would rather try to
rework ArgumnentProxy (or get rid of it) to better match the general mathed
structure: Each inset is self contained and does not need to know anything
about other insets.
Georg
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 898eab3..9ff7fc2 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -2130,8 +2130,10 @@ void Cursor::handleFont(string const & font)
// cursor in between. split cell
MathData::iterator bt = cell().begin();
MathAtom at = createInsetMath(from_utf8(font), buffer());
- at.nucleus()->cell(0) = MathData(buffer(), bt, bt + pos());
- cell().erase(bt, bt + pos());
+ MathData::iterator it = bt;
+ advance(it, pos());
+ at.nucleus()->cell(0) = MathData(buffer(), bt, it);
+ cell().erase(bt, it);
popBackward();
plainInsert(at);
}
diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp
index 6333892..6536d54 100644
--- a/src/CutAndPaste.cpp
+++ b/src/CutAndPaste.cpp
@@ -1363,7 +1363,11 @@ docstring grabSelection(Cursor const & cur)
if (i1.inset().asInsetMath()) {
MathData::const_iterator it = i1.cell().begin();
Buffer * buf = cur.buffer();
- return asString(MathData(buf, it + i1.pos(), it + i2.pos()));
+ MathData::const_iterator it1 = it;
+ advance(it1, i1.pos());
+ MathData::const_iterator it2 = it;
+ advance(it2, i2.pos());
+ return asString(MathData(buf, it1, it2));
} else {
return from_ascii("unknown selection 1");
}
diff --git a/src/DocIterator.cpp b/src/DocIterator.cpp
index 750479f..a821455 100644
--- a/src/DocIterator.cpp
+++ b/src/DocIterator.cpp
@@ -317,9 +317,11 @@ void DocIterator::forwardPos()
if (tip.pos() != tip.lastpos()) {
// move into an inset to the right if possible
Inset * n = 0;
- if (inMathed())
- n = (tip.cell().begin() + tip.pos())->nucleus();
- else
+ if (inMathed()) {
+ MathData::iterator it = tip.cell().begin();
+ advance(it, tip.pos());
+ n = it->nucleus();
+ } else
n = paragraph().getInset(tip.pos());
if (n && n->isActive()) {
//lyxerr << "... descend" << endl;
@@ -436,9 +438,11 @@ void DocIterator::backwardPos()
// move into an inset to the left if possible
Inset * n = 0;
- if (inMathed())
- n = (top().cell().begin() + top().pos())->nucleus();
- else
+ if (inMathed()) {
+ MathData::iterator it = top().cell().begin();
+ advance(it, top().pos());
+ n = it->nucleus();
+ } else
n = paragraph().getInset(top().pos());
if (n && n->isActive()) {
push_back(CursorSlice(*n));
@@ -535,9 +539,11 @@ bool DocIterator::fixIfBroken()
break;
} else if (i != n - 1 && cs.pos() != cs.lastpos()) {
// get inset which is supposed to be in the next slice
- if (cs.inset().inMathed())
- inset = (cs.cell().begin() + cs.pos())->nucleus();
- else if (Inset * csInset = cs.paragraph().getInset(cs.pos()))
+ if (cs.inset().inMathed()) {
+ MathData::iterator it = cs.cell().begin();
+ advance(it, cs.pos());
+ inset = it->nucleus();
+ } else if (Inset * csInset = cs.paragraph().getInset(cs.pos()))
inset = csInset;
else {
// there are slices left, so there must be another inset
diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index f3b3eef..5db671e 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -1025,12 +1025,13 @@ docstring stringifyFromCursor(DocIterator const & cur, int len)
docstring s;
CursorSlice cs = cur.top();
MathData md = cs.cell();
- MathData::const_iterator it_end =
- (( len == -1 || cs.pos() + len > int(md.size()))
- ? md.end()
- : md.begin() + cs.pos() + len );
- for (MathData::const_iterator it = md.begin() + cs.pos();
- it != it_end; ++it)
+ MathData::const_iterator it_end = md.end();
+ if (!( len == -1 || cs.pos() + len > int(md.size()))) {
+ it_end = md.begin();
+ advance(it_end, cs.pos() + len);
+ }
+ MathData::const_iterator it = md.begin();
+ for (advance(it, cs.pos()); it != it_end; ++it)
s = s + asString(*it);
LYXERR(Debug::FIND, "Stringified math: '" << s << "'");
return s;
@@ -1083,12 +1084,13 @@ docstring latexifyFromCursor(DocIterator const & cur, int len)
CursorSlice const & cs = cur.top();
MathData md = cs.cell();
- MathData::const_iterator it_end =
- ((len == -1 || cs.pos() + len > int(md.size()))
- ? md.end()
- : md.begin() + cs.pos() + len);
- for (MathData::const_iterator it = md.begin() + cs.pos();
- it != it_end; ++it)
+ MathData::const_iterator it_end = md.end();
+ if (!( len == -1 || cs.pos() + len > int(md.size()))) {
+ it_end = md.begin();
+ advance(it_end, cs.pos() + len);
+ }
+ MathData::const_iterator it = md.begin();
+ for (advance(it, cs.pos()); it != it_end; ++it)
ods << asString(*it);
// Retrieve the math environment type, and add '$' or '$]'
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 53a16e8..c0a82a1 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -91,9 +91,10 @@ namespace {
// used for "intelligent splitting"
size_t firstRelOp(MathData const & ar)
{
- for (MathData::const_iterator it = ar.begin(); it != ar.end(); ++it)
+ size_t i = 0;
+ for (MathData::const_iterator it = ar.begin(); it != ar.end(); ++it, ++i)
if ((*it)->isRelOp())
- return it - ar.begin();
+ return i;
return ar.size();
}
@@ -1067,7 +1068,9 @@ void InsetMathHull::splitTo2Cols()
for (row_type row = 0; row < nrows(); ++row) {
idx_type const i = 2 * row;
pos_type pos = firstRelOp(cell(i));
- cell(i + 1) = MathData(buffer_, cell(i).begin() + pos, cell(i).end());
+ MathData::iterator it = cell(i).begin();
+ advance(it, pos);
+ cell(i + 1) = MathData(buffer_, it, cell(i).end());
cell(i).erase(pos, cell(i).size());
}
}
@@ -1082,7 +1085,9 @@ void InsetMathHull::splitTo3Cols()
for (row_type row = 0; row < nrows(); ++row) {
idx_type const i = 3 * row + 1;
if (!cell(i).empty()) {
- cell(i + 1) = MathData(buffer_, cell(i).begin() + 1, cell(i).end());
+ MathData::iterator it = cell(i).begin();
+ ++it;
+ cell(i + 1) = MathData(buffer_, it, cell(i).end());
cell(i).erase(1, cell(i).size());
}
}
@@ -1357,7 +1362,9 @@ void InsetMathHull::doExtern(Cursor & cur, FuncRequest & func)
ar = cur.cell();
lyxerr << "use whole cell: " << ar << endl;
} else {
- ar = MathData(buffer_, cur.cell().begin() + pos + 1, cur.cell().end());
+ MathData::iterator it = cur.cell().begin();
+ advance(it, pos + 1);
+ ar = MathData(buffer_, it, cur.cell().end());
lyxerr << "use partial cell form pos: " << pos << endl;
}
cur.cell().append(eq);
diff --git a/src/mathed/MathData.cpp b/src/mathed/MathData.cpp
index 1b79f0c..0afd2f1 100644
--- a/src/mathed/MathData.cpp
+++ b/src/mathed/MathData.cpp
@@ -69,14 +69,18 @@ MathAtom const & MathData::operator[](pos_type pos) const
void MathData::insert(size_type pos, MathAtom const & t)
{
LBUFERR(pos <= size());
- base_type::insert(begin() + pos, t);
+ iterator it = begin();
+ advance(it, pos);
+ base_type::insert(it, t);
}
void MathData::insert(size_type pos, MathData const & ar)
{
LBUFERR(pos <= size());
- base_type::insert(begin() + pos, ar.begin(), ar.end());
+ iterator it = begin();
+ advance(it, pos);
+ base_type::insert(it, ar.begin(), ar.end());
}
@@ -107,7 +111,11 @@ void MathData::erase(iterator pos)
void MathData::erase(size_type pos1, size_type pos2)
{
- base_type::erase(begin() + pos1, begin() + pos2);
+ iterator it1 = begin();
+ advance(it1, pos1);
+ iterator it2 = begin();
+ advance(it2, pos2);
+ base_type::erase(it1, it2);
}
@@ -148,7 +156,8 @@ bool MathData::matchpart(MathData const & ar, pos_type pos) const
{
if (size() < ar.size() + pos)
return false;
- const_iterator it = begin() + pos;
+ const_iterator it = begin();
+ advance(it, pos);
for (const_iterator jt = ar.begin(); jt != ar.end(); ++jt, ++it)
if (asString(*it) != asString(*jt))
return false;
@@ -676,7 +685,7 @@ void MathData::attachMacroParameters(Cursor * cur,
}
// remove them from the MathData
- erase(begin() + macroPos + 1, begin() + p);
+ erase(macroPos + 1, p);
// cursor outside this MathData?
if (thisSlice == -1)
@@ -744,7 +753,11 @@ void MathData::collectOptionalParameters(Cursor * cur,
}
// add everything between [ and ] as optional argument
- MathData optarg(buf, begin() + pos + 1, begin() + right);
+ iterator it1 = begin();
+ advance(it1, pos + 1);
+ iterator it2 = begin();
+ advance(it2, right);
+ MathData optarg(buf, it1, it2);
// a brace?
bool brace = false;
@@ -859,7 +872,8 @@ int MathData::pos2x(BufferView const * bv, size_type pos, int glue) const
size_type target = min(pos, size());
CoordCacheBase<Inset> const & coords = bv->coordCache().getInsets();
for (size_type i = 0; i < target; ++i) {
- const_iterator it = begin() + i;
+ const_iterator it = begin();
+ advance(it, i);
if ((*it)->getChar() == ' ')
x += glue;
//lyxerr << "char: " << (*it)->getChar()
@@ -883,7 +897,7 @@ MathData::size_type MathData::x2pos(BufferView const * bv, int targetx, int glue
int currx = 0;
CoordCacheBase<Inset> const & coords = bv->coordCache().getInsets();
// find first position after targetx
- for (; currx < targetx && it < end(); ++it) {
+ for (; currx < targetx && it != end(); ++it) {
lastx = currx;
if ((*it)->getChar() == ' ')
currx += glue;
@@ -906,7 +920,10 @@ MathData::size_type MathData::x2pos(BufferView const * bv, int targetx, int glue
--it;
}
- return it - begin();
+ for (size_t i = 0; true; ++i, --it)
+ if (it == begin())
+ return i;
+ return 0;
}
diff --git a/src/mathed/MathData.h b/src/mathed/MathData.h
index e216941..8bb9c930 100644
--- a/src/mathed/MathData.h
+++ b/src/mathed/MathData.h
@@ -20,6 +20,7 @@
#include "OutputEnums.h"
+#include "support/RandomAccessList.h"
#include "support/strfwd.h"
#include <cstddef>
@@ -43,10 +44,10 @@ class TextMetricsInfo;
class TextPainter;
-class MathData : private std::vector<MathAtom> {
+class MathData : private RandomAccessList<MathAtom> {
public:
/// re-use inhertited stuff
- typedef std::vector<MathAtom> base_type;
+ typedef RandomAccessList<MathAtom> base_type;
using base_type::const_iterator;
using base_type::iterator;
using base_type::size_type;
diff --git a/src/mathed/MathExtern.cpp b/src/mathed/MathExtern.cpp
index a943dac..8a769e8 100644
--- a/src/mathed/MathExtern.cpp
+++ b/src/mathed/MathExtern.cpp
@@ -185,7 +185,9 @@ void extractStrings(MathData & ar)
for (size_t i = 0; i < ar.size(); ++i) {
if (!ar[i]->asCharInset())
continue;
- docstring s = charSequence(ar.begin() + i, ar.end());
+ MathData::iterator it = ar.begin();
+ advance(it, i);
+ docstring s = charSequence(it, ar.end());
ar[i] = MathAtom(new InsetMathString(s));
ar.erase(i + 1, i + s.size());
}
@@ -323,14 +325,17 @@ void replaceNested(
continue;
// search end of sequence
- MathData::iterator it = ar.begin() + i;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
MathData::iterator jt = endNestSearch(it, ar.end(), testOpen, testClose);
if (jt == ar.end())
continue;
// replace the original stuff by the new inset
- ar[i] = replaceArg(MathData(buf, it + 1, jt));
- ar.erase(it + 1, jt + 1);
+ ++it;
+ ar[i] = replaceArg(MathData(buf, it, jt));
+ ++jt;
+ ar.erase(it, jt);
}
}
@@ -466,7 +471,9 @@ void extractNumbers(MathData & ar)
if (!isDigitOrSimilar(ar[i]->asCharInset()->getChar()))
continue;
- docstring s = digitSequence(ar.begin() + i, ar.end());
+ MathData::iterator it = ar.begin();
+ advance(it, i);
+ docstring s = digitSequence(it, ar.end());
ar[i] = MathAtom(new InsetMathNumber(s));
ar.erase(i + 1, i + s.size());
@@ -549,8 +556,10 @@ void extractFunctions(MathData & ar, ExternalMath kind)
//lyxerr << "\nFunctions from: " << ar << endl;
for (size_t i = 0; i + 1 < ar.size(); ++i) {
- MathData::iterator it = ar.begin() + i;
- MathData::iterator jt = it + 1;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
+ MathData::iterator jt = it;
+ ++jt;
docstring name;
// is it a function?
@@ -604,7 +613,7 @@ void extractFunctions(MathData & ar, ExternalMath kind)
*it = MathAtom(p.release());
// remove the source of the argument from the array
- ar.erase(it + 1, st);
+ ar.erase(jt, st);
// re-insert exponent
ar.insert(i + 1, exp);
@@ -664,7 +673,8 @@ void extractIntegrals(MathData & ar, ExternalMath kind)
//lyxerr << "\nIntegrals from: " << ar << endl;
for (size_t i = 0; i + 1 < ar.size(); ++i) {
- MathData::iterator it = ar.begin() + i;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
// search 'd'
MathData::iterator jt =
@@ -686,13 +696,16 @@ void extractIntegrals(MathData & ar, ExternalMath kind)
p->cell(2) = (*it)->asScriptInset()->down();
p->cell(3) = (*it)->asScriptInset()->up();
}
- p->cell(0) = MathData(buf, it + 1, jt);
+ MathData::iterator it1 = it;
+ ++it1;
+ p->cell(0) = MathData(buf, it1, jt);
// use the "thing" behind the 'd' as differential
- MathData::iterator tt = extractArgument(p->cell(1), jt + 1, ar.end(), kind);
+ ++jt;
+ MathData::iterator tt = extractArgument(p->cell(1), jt, ar.end(), kind);
// remove used parts
- ar.erase(it + 1, tt);
+ ar.erase(it1, tt);
*it = MathAtom(p.release());
}
//lyxerr << "\nIntegrals to: " << ar << endl;
@@ -757,7 +770,8 @@ void extractSums(MathData & ar)
//lyxerr << "\nSums from: " << ar << endl;
for (size_t i = 0; i + 1 < ar.size(); ++i) {
- MathData::iterator it = ar.begin() + i;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
// is this a sum name?
if (!testSum(ar[i]))
@@ -777,7 +791,8 @@ void extractSums(MathData & ar)
// we found a '=', use everything in front of that as index,
// and everything behind as lower index
p->cell(1) = MathData(buf, ar.begin(), xt);
- p->cell(2) = MathData(buf, xt + 1, ar.end());
+ ++xt;
+ p->cell(2) = MathData(buf, xt, ar.end());
} else {
// use everything as summation index, don't use scripts.
p->cell(1) = ar;
@@ -789,10 +804,12 @@ void extractSums(MathData & ar)
p->cell(3) = sub->up();
// use something behind the script as core
- MathData::iterator tt = extractTerm(p->cell(0), it + 1, ar.end());
+ MathData::iterator jt = it;
+ ++jt;
+ MathData::iterator tt = extractTerm(p->cell(0), jt, ar.end());
// cleanup
- ar.erase(it + 1, tt);
+ ar.erase(jt, tt);
*it = MathAtom(p.release());
}
//lyxerr << "\nSums to: " << ar << endl;
@@ -839,7 +856,8 @@ void extractDiff(MathData & ar)
Buffer * buf = ar.buffer();
//lyxerr << "\nDiffs from: " << ar << endl;
for (size_t i = 0; i < ar.size(); ++i) {
- MathData::iterator it = ar.begin() + i;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
// is this a "differential fraction"?
if (!testDiffFrac(*it))
@@ -855,7 +873,8 @@ void extractDiff(MathData & ar)
auto_ptr<InsetMathDiff> diff(new InsetMathDiff(buf));
// collect function, let jt point behind last used item
- MathData::iterator jt = it + 1;
+ MathData::iterator jt = it;
+ ++jt;
//int n = 1;
MathData numer(f->cell(0));
splitScripts(numer);
@@ -863,15 +882,19 @@ void extractDiff(MathData & ar)
// this is something like d^n f(x) / d... or d^n / d...
// FIXME
//n = 1;
- if (numer.size() > 2)
- diff->cell(0) = MathData(buf, numer.begin() + 2, numer.end());
- else
+ if (numer.size() > 2) {
+ MathData::iterator nt = numer.begin();
+ advance(nt, 2);
+ diff->cell(0) = MathData(buf, nt, numer.end());
+ } else
jt = extractTerm(diff->cell(0), jt, ar.end());
} else {
// simply d f(x) / d... or d/d...
- if (numer.size() > 1)
- diff->cell(0) = MathData(buf, numer.begin() + 1, numer.end());
- else
+ if (numer.size() > 1) {
+ MathData::iterator nt = numer.begin();
+ ++nt;
+ diff->cell(0) = MathData(buf, nt, numer.end());
+ } else
jt = extractTerm(diff->cell(0), jt, ar.end());
}
@@ -880,11 +903,14 @@ void extractDiff(MathData & ar)
splitScripts(denom);
for (MathData::iterator dt = denom.begin(); dt != denom.end();) {
// find the next 'd'
+ MathData::iterator dt1 = dt;
+ ++dt1;
MathData::iterator et
- = find_if(dt + 1, denom.end(), &testDiffItem);
+ = find_if(dt1, denom.end(), &testDiffItem);
// point before this
- MathData::iterator st = et - 1;
+ MathData::iterator st = et;
+ --st;
InsetMathScript const * script = (*st)->asScriptInset();
if (script && script->hasUp()) {
// things like d.../dx^n
@@ -892,17 +918,19 @@ void extractDiff(MathData & ar)
if (extractNumber(script->up(), mult)) {
//lyxerr << "mult: " << mult << endl;
for (int i = 0; i < mult; ++i)
- diff->addDer(MathData(buf, dt + 1, st));
+ diff->addDer(MathData(buf, dt1, st));
}
} else {
// just d.../dx
- diff->addDer(MathData(buf, dt + 1, et));
+ diff->addDer(MathData(buf, dt1, et));
}
dt = et;
}
// cleanup
- ar.erase(it + 1, jt);
+ MathData::iterator it1 = it;
+ ++it1;
+ ar.erase(it1, jt);
*it = MathAtom(diff.release());
}
//lyxerr << "\nDiffs to: " << ar << endl;
@@ -928,7 +956,8 @@ void extractLims(MathData & ar)
Buffer * buf = ar.buffer();
//lyxerr << "\nLimits from: " << ar << endl;
for (size_t i = 0; i < ar.size(); ++i) {
- MathData::iterator it = ar.begin() + i;
+ MathData::iterator it = ar.begin();
+ advance(it, i);
// must be a script inset with a subscript (without superscript)
InsetMathScript const * sub = (*it)->asScriptInset();
@@ -947,14 +976,17 @@ void extractLims(MathData & ar)
// the -> splits the subscript int x and x0
MathData x = MathData(buf, s.begin(), st);
- MathData x0 = MathData(buf, st + 1, s.end());
+ ++st;
+ MathData x0 = MathData(buf, st, s.end());
// use something behind the script as core
MathData f;
- MathData::iterator tt = extractTerm(f, it + 1, ar.end());
+ MathData::iterator it1 = it;
+ ++it1;
+ MathData::iterator tt = extractTerm(f, it1, ar.end());
// cleanup
- ar.erase(it + 1, tt);
+ ar.erase(it1, tt);
// create a proper inset as replacement
*it = MathAtom(new InsetMathLim(buf, f, x, x0));
