qdoc: Simplification of target ref construction
authorMartin Smith <martin.smith@digia.com>
Fri, 5 Oct 2012 12:35:00 +0000 (14:35 +0200)
committerThe Qt Project <gerrit-noreply@qt-project.org>
Mon, 8 Oct 2012 07:12:20 +0000 (09:12 +0200)
This change is being done to simplify qdoc,
but the motivation was to fix a segfault in
qdoc when running the release version of
qdoc on linux. The change improves qdoc by
simplifying the code whether it fixes the
segfault or not.

Change-Id: I2c865f7f1e2a44763aa7349d1bd739ad562f4029
Reviewed-by: Jerome Pasion <jerome.pasion@digia.com>
src/tools/qdoc/ditaxmlgenerator.cpp
src/tools/qdoc/ditaxmlgenerator.h
src/tools/qdoc/htmlgenerator.cpp
src/tools/qdoc/htmlgenerator.h
src/tools/qdoc/qdocdatabase.cpp
src/tools/qdoc/qdocdatabase.h

index 5211b27..8c1f57a 100644 (file)
@@ -3766,15 +3766,6 @@ QString DitaXmlGenerator::linkForNode(const Node* node, const Node* relative)
     return link;
 }
 
-QString DitaXmlGenerator::refForAtom(Atom* atom, const Node* /* node */)
-{
-    if (atom->type() == Atom::SectionLeft)
-        return Doc::canonicalTitle(Text::sectionHeading(atom).toString());
-    if (atom->type() == Atom::Target)
-        return Doc::canonicalTitle(atom->string());
-    return QString();
-}
-
 void DitaXmlGenerator::generateFullName(const Node* apparentNode,
                                         const Node* relative,
                                         const Node* actualNode)
@@ -3855,7 +3846,7 @@ QString DitaXmlGenerator::getLink(const Atom* atom, const Node* relative, const
         else
             path.append(atom->string());
 
-        Atom* targetAtom = 0;
+        QString ref;
         QString first = path.first().trimmed();
 
         if (first.isEmpty()) {
@@ -3869,7 +3860,7 @@ QString DitaXmlGenerator::getLink(const Atom* atom, const Node* relative, const
             if (!*node)
                 *node = qdb_->findDocNodeByTitle(first, relative);
             if (!*node)
-                *node = qdb_->findUnambiguousTarget(first, targetAtom, relative);
+                *node = qdb_->findUnambiguousTarget(first, ref, relative);
         }
 
         if (*node) {
@@ -3903,8 +3894,8 @@ QString DitaXmlGenerator::getLink(const Atom* atom, const Node* relative, const
         }
 
         while (!path.isEmpty()) {
-            targetAtom = qdb_->findTarget(path.first(), *node);
-            if (targetAtom == 0)
+            ref = qdb_->findTarget(path.first(), *node);
+            if (ref.isEmpty())
                 break;
             path.removeFirst();
         }
@@ -3913,10 +3904,10 @@ QString DitaXmlGenerator::getLink(const Atom* atom, const Node* relative, const
             link = linkForNode(*node, relative);
             if (*node && (*node)->subType() == Node::Image)
                 link = "images/used-in-examples/" + link;
-            if (targetAtom) {
+            if (!ref.isEmpty()) {
                 if (link.isEmpty())
                     link = outFileName();
-                QString guid = lookupGuid(link,refForAtom(targetAtom,*node));
+                QString guid = lookupGuid(link, ref);
                 link += QLatin1Char('#') + guid;
             }
             else if (!link.isEmpty() && *node && (link.endsWith(".xml") || link.endsWith(".dita"))) {
@@ -3952,16 +3943,16 @@ void DitaXmlGenerator::generateStatus(const Node* node, CodeMarker* marker)
                  << "using it in new code. See ";
 
             const DocNode *docNode = qdb_->findDocNodeByTitle("Porting To Qt 4");
-            Atom *targetAtom = 0;
+            QString ref;
             if (docNode && node->type() == Node::Class) {
                 QString oldName(node->name());
                 oldName.remove(QLatin1Char('3'));
-                targetAtom = qdb_->findTarget(oldName,docNode);
+                ref = qdb_->findTarget(oldName,docNode);
             }
 
-            if (targetAtom) {
+            if (!ref.isEmpty()) {
                 QString fn = fileName(docNode);
-                QString guid = lookupGuid(fn,refForAtom(targetAtom,docNode));
+                QString guid = lookupGuid(fn, ref);
                 text << Atom(Atom::GuidLink, fn + QLatin1Char('#') + guid);
             }
             else
index 831bbde..7e1f5a0 100644 (file)
@@ -318,7 +318,6 @@ protected:
     virtual QString fileExtension() const;
     virtual QString guidForNode(const Node* node);
     virtual QString linkForNode(const Node* node, const Node* relative);
-    virtual QString refForAtom(Atom* atom, const Node* node);
 
     void writeXrefListItem(const QString& link, const QString& text);
     QString fullQualification(const Node* n);
index c3cf38b..7eb06ce 100644 (file)
@@ -3344,19 +3344,6 @@ QString HtmlGenerator::linkForNode(const Node *node, const Node *relative)
     return link;
 }
 
-QString HtmlGenerator::refForAtom(Atom *atom, const Node * /* node */)
-{
-    if (atom->type() == Atom::SectionLeft) {
-        return Doc::canonicalTitle(Text::sectionHeading(atom).toString());
-    }
-    else if (atom->type() == Atom::Target) {
-        return Doc::canonicalTitle(atom->string());
-    }
-    else {
-        return QString();
-    }
-}
-
 void HtmlGenerator::generateFullName(const Node *apparentNode, const Node *relative, const Node *actualNode)
 {
     if (actualNode == 0)
@@ -3519,7 +3506,7 @@ QString HtmlGenerator::getLink(const Atom *atom, const Node *relative, const Nod
             path.append(atom->string());
         }
 
-        Atom *targetAtom = 0;
+        QString ref;
         QString first = path.first().trimmed();
         if (first.isEmpty()) {
             *node = relative;
@@ -3539,7 +3526,7 @@ QString HtmlGenerator::getLink(const Atom *atom, const Node *relative, const Nod
                 *node = qdb_->findDocNodeByTitle(first, relative);
             }
             if (!*node) {
-                *node = qdb_->findUnambiguousTarget(first, targetAtom, relative);
+                *node = qdb_->findUnambiguousTarget(first, ref, relative);
             }
         }
         if (*node) {
@@ -3589,12 +3576,11 @@ QString HtmlGenerator::getLink(const Atom *atom, const Node *relative, const Nod
           In that case, The node *node points to represents a
           qdoc page, so the link will ultimately point to some
           target on that page. This loop finds that target on
-          the page that *node represents. targetAtom is that
-          target.
+          the page that *node represents. ref is that target.
          */
         while (!path.isEmpty()) {
-            targetAtom = qdb_->findTarget(path.first(), *node);
-            if (targetAtom == 0)
+            ref = qdb_->findTarget(path.first(), *node);
+            if (ref.isEmpty())
                 break;
             path.removeFirst();
         }
@@ -3609,8 +3595,9 @@ QString HtmlGenerator::getLink(const Atom *atom, const Node *relative, const Nod
             link = linkForNode(*node, relative);
             if (*node && (*node)->subType() == Node::Image)
                 link = "images/used-in-examples/" + link;
-            if (targetAtom)
-                link += QLatin1Char('#') + refForAtom(targetAtom, *node);
+            if (!ref.isEmpty()) {
+                link += QLatin1Char('#') + ref;
+            }
         }
     }
     return link;
@@ -3638,16 +3625,15 @@ void HtmlGenerator::generateStatus(const Node *node, CodeMarker *marker)
                  << "using it in new code. See ";
 
             const DocNode *docNode = qdb_->findDocNodeByTitle("Porting To Qt 4");
-            Atom *targetAtom = 0;
+            QString ref;
             if (docNode && node->type() == Node::Class) {
                 QString oldName(node->name());
                 oldName.remove(QLatin1Char('3'));
-                targetAtom = qdb_->findTarget(oldName, docNode);
+                ref = qdb_->findTarget(oldName, docNode);
             }
 
-            if (targetAtom) {
-                text << Atom(Atom::Link, linkForNode(docNode, node) + QLatin1Char('#') +
-                             refForAtom(targetAtom, docNode));
+            if (!ref.isEmpty()) {
+                text << Atom(Atom::Link, linkForNode(docNode, node) + QLatin1Char('#') + ref);
             }
             else
                 text << Atom(Atom::Link, "Porting to Qt 4");
index be9fc70..c4af3c4 100644 (file)
@@ -104,7 +104,6 @@ protected:
     virtual QString fileExtension() const;
     virtual QString refForNode(const Node *node);
     virtual QString linkForNode(const Node *node, const Node *relative);
-    virtual QString refForAtom(Atom *atom, const Node *node);
 
     void generateManifestFile(QString manifest, QString element);
 
index 9d3a8be..8780c67 100644 (file)
@@ -39,6 +39,7 @@
 **
 ****************************************************************************/
 
+#include "generator.h"
 #include "atom.h"
 #include "tree.h"
 #include "qdocdatabase.h"
@@ -639,62 +640,84 @@ const Node* QDocDatabase::findNodeForTarget(const QString& target, const Node* r
 void QDocDatabase::insertTarget(const QString& name, Node* node, int priority)
 {
     Target target;
-    target.node = node;
-    target.priority = priority;
-    target.atom = new Atom(Atom::Target, name);
-    targetHash_.insert(name, target);
+    target.node_ = node;
+    target.priority_ = priority;
+    Atom a = Atom(Atom::Target, name);
+    target.ref_ = refForAtom(&a);
+    targetMultiMap_.insert(name, target);
 }
 
-static const int NumSuffixes = 3;
-static const char*  const suffixes[NumSuffixes] = { "", "s", "es" };
-
 /*!
   This function searches for a \a target anchor node. If it
-  finds one, it sets \a atom from the found node and returns
-  the found node.
+  finds one, it sets \a ref and returns the found node.
  */
 const Node*
-QDocDatabase::findUnambiguousTarget(const QString& target, Atom *&atom, const Node* relative) const
+QDocDatabase::findUnambiguousTarget(const QString& target, QString& ref, const Node* relative) const
 {
-    Target bestTarget = {0, 0, INT_MAX};
+    Target bestTarget;
     int numBestTargets = 0;
     QList<Target> bestTargetList;
 
-    for (int pass = 0; pass < NumSuffixes; ++pass) {
-        TargetHash::const_iterator i = targetHash_.constFind(Doc::canonicalTitle(target + suffixes[pass]));
-        if (i != targetHash_.constEnd()) {
-            TargetHash::const_iterator j = i;
-            do {
-                const Target& candidate = j.value();
-                if (candidate.priority < bestTarget.priority) {
-                    bestTarget = candidate;
-                    bestTargetList.clear();
-                    bestTargetList.append(candidate);
-                    numBestTargets = 1;
-                } else if (candidate.priority == bestTarget.priority) {
-                    bestTargetList.append(candidate);
-                    ++numBestTargets;
-                }
-                ++j;
-            } while (j != targetHash_.constEnd() && j.key() == i.key());
-
-            if (numBestTargets == 1) {
-                atom = bestTarget.atom;
-                return bestTarget.node;
+    bool debug = false;
+    if (target == "Manager" && Generator::debugging())
+        debug = true;
+
+    QString key = Doc::canonicalTitle(target);
+    TargetMultiMap::const_iterator i = targetMultiMap_.constFind(key);
+    if (i != targetMultiMap_.constEnd()) {
+        if (debug)
+            qDebug() << "DEBUG: A";
+        TargetMultiMap::const_iterator j = i;
+        do {
+            const Target& candidate = j.value();
+            if (candidate.priority_ < bestTarget.priority_) {
+                if (debug)
+                    qDebug() << "DEBUG: B";
+                bestTarget = candidate;
+                bestTargetList.clear();
+                bestTargetList.append(candidate);
+                numBestTargets = 1;
+            } else if (candidate.priority_ == bestTarget.priority_) {
+                if (debug)
+                    qDebug() << "DEBUG: C";
+                bestTargetList.append(candidate);
+                ++numBestTargets;
             }
-            else if (bestTargetList.size() > 1) {
-                if (relative && !relative->qmlModuleIdentifier().isEmpty()) {
-                    for (int i=0; i<bestTargetList.size(); ++i) {
-                        const Node* n = bestTargetList.at(i).node;
-                        if (n && relative->qmlModuleIdentifier() == n->qmlModuleIdentifier()) {
-                            atom = bestTargetList.at(i).atom;
-                            return n;
-                        }
+            ++j;
+        } while (j != targetMultiMap_.constEnd() && j.key() == i.key());
+
+        if (debug)
+            qDebug() << "DEBUG: D";
+        if (numBestTargets == 1) {
+            if (debug)
+                qDebug() << "DEBUG: E";
+            ref = bestTarget.ref_;
+            return bestTarget.node_;
+        }
+        else if (bestTargetList.size() > 1) {
+            if (debug)
+                qDebug() << "DEBUG: F";
+            if (relative && !relative->qmlModuleIdentifier().isEmpty()) {
+                if (debug)
+                    qDebug() << "DEBUG: G";
+                for (int i=0; i<bestTargetList.size(); ++i) {
+                    if (debug)
+                        qDebug() << "DEBUG: H";
+                    const Node* n = bestTargetList.at(i).node_;
+                    if (debug)
+                        qDebug() << "DEBUG: I";
+                    if (n && relative->qmlModuleIdentifier() == n->qmlModuleIdentifier()) {
+                        if (debug)
+                            qDebug() << "DEBUG: J";
+                        ref = bestTargetList.at(i).ref_;
+                        return n;
                     }
                 }
             }
         }
     }
+    if (debug)
+        qDebug() << "DEBUG: K";
     return 0;
 }
 
@@ -705,77 +728,74 @@ QDocDatabase::findUnambiguousTarget(const QString& target, Atom *&atom, const No
  */
 const DocNode* QDocDatabase::findDocNodeByTitle(const QString& title, const Node* relative) const
 {
-    for (int pass = 0; pass < NumSuffixes; ++pass) {
-        DocNodeHash::const_iterator i = docNodesByTitle_.constFind(Doc::canonicalTitle(title + suffixes[pass]));
-        if (i != docNodesByTitle_.constEnd()) {
-            if (relative && !relative->qmlModuleIdentifier().isEmpty()) {
-                const DocNode* dn = i.value();
-                InnerNode* parent = dn->parent();
-                if (parent && parent->type() == Node::Document && parent->subType() == Node::Collision) {
-                    const NodeList& nl = parent->childNodes();
-                    NodeList::ConstIterator it = nl.constBegin();
-                    while (it != nl.constEnd()) {
-                        if ((*it)->qmlModuleIdentifier() == relative->qmlModuleIdentifier()) {
-                            /*
-                              By returning here, we avoid printing all the duplicate
-                              header warnings, which are not really duplicates now,
-                              because of the QML module identifier being used as a
-                              namespace qualifier.
-                             */
-                            dn = static_cast<const DocNode*>(*it);
-                            return dn;
-                        }
-                        ++it;
+    QString key = Doc::canonicalTitle(title);
+    DocNodeMultiMap::const_iterator i = docNodesByTitle_.constFind(key);
+    if (i != docNodesByTitle_.constEnd()) {
+        if (relative && !relative->qmlModuleIdentifier().isEmpty()) {
+            const DocNode* dn = i.value();
+            InnerNode* parent = dn->parent();
+            if (parent && parent->type() == Node::Document && parent->subType() == Node::Collision) {
+                const NodeList& nl = parent->childNodes();
+                NodeList::ConstIterator it = nl.constBegin();
+                while (it != nl.constEnd()) {
+                    if ((*it)->qmlModuleIdentifier() == relative->qmlModuleIdentifier()) {
+                        /*
+                          By returning here, we avoid printing all the duplicate
+                          header warnings, which are not really duplicates now,
+                          because of the QML module identifier being used as a
+                          namespace qualifier.
+                        */
+                        dn = static_cast<const DocNode*>(*it);
+                        return dn;
                     }
+                    ++it;
                 }
             }
-            /*
-              Reporting all these duplicate section titles is probably
-              overkill. We should report the duplicate file and let
-              that suffice.
-             */
-            DocNodeHash::const_iterator j = i;
-            ++j;
-            if (j != docNodesByTitle_.constEnd() && j.key() == i.key()) {
-                QList<Location> internalLocations;
-                while (j != docNodesByTitle_.constEnd()) {
-                    if (j.key() == i.key() && j.value()->url().isEmpty())
-                        internalLocations.append(j.value()->location());
-                    ++j;
-                }
-                if (internalLocations.size() > 0) {
-                    i.value()->location().warning(tr("This page exists in more than one file: \"%1\"").arg(title));
-                    foreach (const Location &location, internalLocations)
-                        location.warning(tr("[It also exists here]"));
-                }
+        }
+        /*
+          Reporting all these duplicate section titles is probably
+          overkill. We should report the duplicate file and let
+          that suffice.
+        */
+        DocNodeMultiMap::const_iterator j = i;
+        ++j;
+        if (j != docNodesByTitle_.constEnd() && j.key() == i.key()) {
+            QList<Location> internalLocations;
+            while (j != docNodesByTitle_.constEnd()) {
+                if (j.key() == i.key() && j.value()->url().isEmpty())
+                    internalLocations.append(j.value()->location());
+                ++j;
+            }
+            if (internalLocations.size() > 0) {
+                i.value()->location().warning(tr("This page exists in more than one file: \"%1\"").arg(title));
+                foreach (const Location &location, internalLocations)
+                    location.warning(tr("[It also exists here]"));
             }
-            return i.value();
         }
+        return i.value();
     }
     return 0;
 }
 
 /*!
   This function searches for a node with a canonical title
-  constructed from \a target and each of the possible suffixes.
-  If the node it finds is \a node, it returns the Atom from that
-  node. Otherwise it returns null.
- */
-Atom* QDocDatabase::findTarget(const QString& target, const Node* node) const
-{
-    for (int pass = 0; pass < NumSuffixes; ++pass) {
-        QString key = Doc::canonicalTitle(target + suffixes[pass]);
-        TargetHash::const_iterator i = targetHash_.constFind(key);
-
-        if (i != targetHash_.constEnd()) {
-            do {
-                if (i.value().node == node)
-                    return i.value().atom;
-                ++i;
-            } while (i != targetHash_.constEnd() && i.key() == key);
-        }
+  constructed from \a target. If the node it finds is \a node,
+  it returns the ref from that node. Otherwise it returns an
+  empty string.
+ */
+QString QDocDatabase::findTarget(const QString& target, const Node* node) const
+{
+    QString key = Doc::canonicalTitle(target);
+    TargetMultiMap::const_iterator i = targetMultiMap_.constFind(key);
+
+    if (i != targetMultiMap_.constEnd()) {
+        do {
+            if (i.value().node_ == node)
+                return i.value().ref_;
+            ++i;
+        } while (i != targetMultiMap_.constEnd() && i.key() == key);
     }
-    return 0;
+    return QString();
 }
 
 /*!
@@ -787,8 +807,10 @@ void QDocDatabase::resolveTargets(InnerNode* root)
     foreach (Node* child, root->childNodes()) {
         if (child->type() == Node::Document) {
             DocNode* node = static_cast<DocNode*>(child);
-            if (!node->title().isEmpty())
-                docNodesByTitle_.insert(Doc::canonicalTitle(node->title()), node);
+            if (!node->title().isEmpty()) {
+                QString key = Doc::canonicalTitle(node->title());
+                docNodesByTitle_.insert(key, node);
+            }
             if (node->subType() == Node::Collision) {
                 resolveTargets(node);
             }
@@ -797,36 +819,40 @@ void QDocDatabase::resolveTargets(InnerNode* root)
         if (child->doc().hasTableOfContents()) {
             const QList<Atom*>& toc = child->doc().tableOfContents();
             Target target;
-            target.node = child;
-            target.priority = 3;
+            target.node_ = child;
+            target.priority_ = 3;
 
             for (int i = 0; i < toc.size(); ++i) {
-                target.atom = toc.at(i);
-                QString title = Text::sectionHeading(target.atom).toString();
-                if (!title.isEmpty())
-                    targetHash_.insert(Doc::canonicalTitle(title), target);
+                target.ref_ = refForAtom(toc.at(i));
+                QString title = Text::sectionHeading(toc.at(i)).toString();
+                if (!title.isEmpty()) {
+                    QString key = Doc::canonicalTitle(title);
+                    targetMultiMap_.insert(key, target);
+                }
             }
         }
         if (child->doc().hasKeywords()) {
             const QList<Atom*>& keywords = child->doc().keywords();
             Target target;
-            target.node = child;
-            target.priority = 1;
+            target.node_ = child;
+            target.priority_ = 1;
 
             for (int i = 0; i < keywords.size(); ++i) {
-                target.atom = keywords.at(i);
-                targetHash_.insert(Doc::canonicalTitle(target.atom->string()), target);
+                target.ref_ = refForAtom(keywords.at(i));
+                QString key = Doc::canonicalTitle(keywords.at(i)->string());
+                targetMultiMap_.insert(key, target);
             }
         }
         if (child->doc().hasTargets()) {
             const QList<Atom*>& toc = child->doc().targets();
             Target target;
-            target.node = child;
-            target.priority = 2;
+            target.node_ = child;
+            target.priority_ = 2;
 
             for (int i = 0; i < toc.size(); ++i) {
-                target.atom = toc.at(i);
-                targetHash_.insert(Doc::canonicalTitle(target.atom->string()), target);
+                target.ref_ = refForAtom(toc.at(i));
+                QString key = Doc::canonicalTitle(toc.at(i)->string());
+                targetMultiMap_.insert(key, target);
             }
         }
     }
@@ -867,4 +893,15 @@ void QDocDatabase::generateIndex(const QString& fileName,
     QDocIndexFiles::destroyQDocIndexFiles();
 }
 
+QString QDocDatabase::refForAtom(const Atom* atom)
+{
+    if (atom) {
+        if (atom->type() == Atom::SectionLeft)
+            return Doc::canonicalTitle(Text::sectionHeading(atom).toString());
+        if (atom->type() == Atom::Target)
+            return Doc::canonicalTitle(atom->string());
+    }
+    return QString();
+}
+
 QT_END_NAMESPACE
index a999954..6d104f3 100644 (file)
@@ -54,7 +54,7 @@ typedef QMap<QString, NodeMap> NodeMapMap;
 typedef QMap<QString, NodeMultiMap> NodeMultiMapMap;
 typedef QMultiMap<QString, Node*> QDocMultiMap;
 typedef QMap<Text, const Node*> TextToNodeMap;
-typedef QMultiHash<QString, DocNode*> DocNodeHash;
+typedef QMultiMap<QString, DocNode*> DocNodeMultiMap;
 
 class Atom;
 class Generator;
@@ -70,11 +70,15 @@ class QDocDatabase
 
     struct Target
     {
-        Node* node;
-        Atom* atom;
-        int priority;
+      public:
+        Target() : node_(0), priority_(INT_MAX) { }
+        bool isEmpty() const { return ref_.isEmpty(); }
+        //void debug(int idx, const QString& key);
+        Node* node_;
+        QString ref_;
+        int priority_;
     };
-    typedef QMultiHash<QString, Target> TargetHash;
+    typedef QMultiMap<QString, Target> TargetMultiMap;
 
   public:
     static QDocDatabase* qdocDB();
@@ -122,6 +126,7 @@ class QDocDatabase
     /* convenience functions
        Many of these will be either eliminated or replaced.
     */
+    QString refForAtom(const Atom* atom);
     Tree* tree() { return tree_; }
     NamespaceNode* treeRoot() { return tree_->root(); }
     void resolveInheritance() { tree_->resolveInheritance(); }
@@ -141,8 +146,8 @@ class QDocDatabase
     }
 
     const DocNode* findDocNodeByTitle(const QString& title, const Node* relative = 0) const;
-    const Node *findUnambiguousTarget(const QString &target, Atom *&atom, const Node* relative) const;
-    Atom *findTarget(const QString &target, const Node *node) const;
+    const Node *findUnambiguousTarget(const QString &target, QString& ref, const Node* relative) const;
+    QString findTarget(const QString &target, const Node *node) const;
     void resolveTargets(InnerNode* root);
 
     FunctionNode* findFunctionNode(const QStringList& parentPath, const FunctionNode* clone) {
@@ -216,8 +221,8 @@ class QDocDatabase
     NodeMultiMapMap         newSinceMaps_;
     NodeMapMap              funcIndex_;
     TextToNodeMap           legaleseTexts_;
-    TargetHash              targetHash_;
-    DocNodeHash             docNodesByTitle_;
+    TargetMultiMap          targetMultiMap_;
+    DocNodeMultiMap         docNodesByTitle_;
 };
 
 QT_END_NAMESPACE