The attached patch is an attempt to get a real start on fixing problems
with recursive includes. As you'll note, there are a couple places I
know still crash, but I wanted to see if anyone had a better idea. On
this approach, we basically have to make sure we NEVER try to recurse
via Buffer::parent() or even Buffer::masterBuffer(). Rather, what you do
is call Buffer::getAncestors() to get the list [parent, grandparent,
etc] and then call some routine on each of those. getAncestors() makes
sure we don't run into an infinite loop [A, B, A, B, etc], but if we
have: A -> B -> A, then A still thinks its ancestors are [B] and B
thinks its are [A]. So even if you've called getAncestors() to get that
list, you must not call some routine on each of them that itself tries
to recurse via getAncestors(). Rather, that must be a simple, one-buffer
routine.
This seems fragile, but I'm not sure what else to try. I had the idea
that somehow somewhere there might be a "global" record of Buffer
inheritance, so that e.g. we would not try to find the masterBuffer()
each time by following parent() but rather by looking at this global
structure. But I don't know how or where to implement that. In a way,
maybe it isn't very hard. You'd only need to update that structure when
an InsetInclude is created or destroyed, or---better---when a Buffer's
parent is modified. I think. And of course the structure itself could be
created in much the way we find this information now (we'd just have to
do it for everything in the BufferList).
Then again, clones will cause problems for that....
Thoughts?
Richard
Index: src/Buffer.h
===================================================================
--- src/Buffer.h (revision 35545)
+++ src/Buffer.h (working copy)
@@ -306,7 +306,10 @@
Buffer const * parent() const;
/** Get the document's master (or \c this if this is not a
- child document)
+ child document).
+ Note that this detects recursive includes, so if A includes B
+ and B includes A, then the master of A is B and the master of
+ B is A. We do not recommend this.
*/
Buffer const * masterBuffer() const;
@@ -605,13 +608,19 @@
void getLanguages(std::set<Language const *> &) const;
/// Update the list of all bibfiles in use (including bibfiles
/// of loaded child documents).
- void updateBibfilesCache(UpdateScope scope = UpdateMaster) const;
+ void updateBibfilesCache() const;
/// Return the list with all bibfiles in use (including bibfiles
/// of loaded child documents).
- support::FileNameList const &
- getBibfilesCache(UpdateScope scope = UpdateMaster) const;
+ support::FileNameList const & getBibfilesCache() const;
///
void collectChildren(ListOfBuffers & children, bool grand_children)
const;
+ /// \return a list of all ancestors of this Buffer. The parent will be
+ /// first, then its parent, etc, so that the master Buffer is last.
+ /// Note that this detects recursive includes and will abort if it finds
+ /// one.
+ ListOfBuffers getAncestors() const;
+ /// utility used by getAncestors()
+ void collectAncestors(ListOfBuffers & ancestors) const;
/// Use the Pimpl idiom to hide the internals.
Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp (revision 35545)
+++ src/Buffer.cpp (working copy)
@@ -1699,15 +1699,8 @@
}
-void Buffer::updateBibfilesCache(UpdateScope scope) const
+void Buffer::updateBibfilesCache() const
{
- // FIXME This is probably unnecssary, given where we call this.
- // If this is a child document, use the parent's cache instead.
- if (parent() && scope != UpdateChildOnly) {
- masterBuffer()->updateBibfilesCache();
- return;
- }
-
d->bibfiles_cache_.clear();
for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
if (it->lyxCode() == BIBTEX_CODE) {
@@ -1718,13 +1711,14 @@
bibfiles.begin(),
bibfiles.end());
} else if (it->lyxCode() == INCLUDE_CODE) {
+ // FIXME Recursive includes still crash this.
InsetInclude & inset =
static_cast<InsetInclude &>(*it);
Buffer const * const incbuf = inset.getChildBuffer();
if (!incbuf)
continue;
support::FileNameList const & bibfiles =
-
incbuf->getBibfilesCache(UpdateChildOnly);
+ incbuf->getBibfilesCache();
if (!bibfiles.empty()) {
d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
bibfiles.begin(),
@@ -1740,10 +1734,14 @@
void Buffer::invalidateBibinfoCache() const
{
d->bibinfo_cache_valid_ = false;
- // also invalidate the cache for the parent buffer
- Buffer const * const pbuf = d->parent();
- if (pbuf)
- pbuf->invalidateBibinfoCache();
+ // also invalidate the cache for our ancestors
+ ListOfBuffers lb = getAncestors();
+ ListOfBuffers::iterator it = lb.begin();
+ ListOfBuffers::iterator const en = lb.end();
+ for (; it != en; it++) {
+ Buffer * bp = *it;
+ bp->d->bibinfo_cache_valid_ = false;
+ }
}
@@ -1751,23 +1749,22 @@
{
d->bibfile_cache_valid_ = false;
d->bibinfo_cache_valid_ = false;
- // also invalidate the cache for the parent buffer
- Buffer const * const pbuf = d->parent();
- if (pbuf)
- pbuf->invalidateBibfileCache();
+ // also invalidate the cache for our ancestors
+ ListOfBuffers lb = getAncestors();
+ ListOfBuffers::iterator it = lb.begin();
+ ListOfBuffers::iterator const en = lb.end();
+ for (; it != en; it++) {
+ Buffer * bp = *it;
+ bp->d->bibinfo_cache_valid_ = false;
+ bp->d->bibfile_cache_valid_ = false;
+ }
}
-support::FileNameList const & Buffer::getBibfilesCache(UpdateScope scope) const
+support::FileNameList const & Buffer::getBibfilesCache() const
{
- // FIXME This is probably unnecessary, given where we call this.
- // If this is a child document, use the master's cache instead.
- Buffer const * const pbuf = masterBuffer();
- if (pbuf != this && scope != UpdateChildOnly)
- return pbuf->getBibfilesCache();
-
if (!d->bibfile_cache_valid_)
- this->updateBibfilesCache(scope);
+ this->updateBibfilesCache();
return d->bibfiles_cache_;
}
@@ -2484,15 +2481,36 @@
}
+ListOfBuffers Buffer::getAncestors() const
+{
+ ListOfBuffers lb;
+ collectAncestors(lb);
+ return lb;
+}
+
+
+void Buffer::collectAncestors(ListOfBuffers & ancestors) const
+{
+ Buffer const * pbuf = parent();
+ if (!pbuf)
+ return;
+ ListOfBuffers::const_iterator it = find(ancestors.begin(),
ancestors.end(), pbuf);
+ if (it != ancestors.end()) {
+ LYXERR0("Recursive include detected!");
+ return;
+ }
+ ancestors.push_back(const_cast<Buffer *>(pbuf));
+ pbuf->collectAncestors(ancestors);
+}
+
+
Buffer const * Buffer::masterBuffer() const
{
- // FIXME Should be make sure we are not in some kind
- // of recursive include? A -> B -> A will crash this.
- Buffer const * const pbuf = d->parent();
- if (!pbuf)
+ ListOfBuffers lb = getAncestors();
+ if (lb.empty())
return this;
-
- return pbuf->masterBuffer();
+ Buffer const * master = lb.back();
+ return master;
}
@@ -2656,6 +2674,7 @@
return data;
// If there is a master buffer, query that
+ // FIXME Recursive Include will crash this....
Buffer const * const pbuf = d->parent();
if (pbuf) {
d->macro_lock = true;
@@ -2887,7 +2906,8 @@
for (; it != end; ++it)
it->first->listMacroNames(macros);
- // call parent
+ // call parents
+ // FIXME Recursive Include will crash this....
Buffer const * const pbuf = d->parent();
if (pbuf)
pbuf->listMacroNames(macros);
