Graph::collectDfs(const qan::Node&) does not include the input node, contradicting its docstring

[cneben]

Summary

qan::Graph::collectDfs(const qan::Node& node, bool collectGroup) documents that the input node is part of the returned vector, but the implementation only walks node.get_out_nodes() and never inserts the input node itself. Callers relying on the documented contract miss the root.

Where

Declaration — src/qanGraph.h:1096-1102:

/*! \brief Synchronously collect all child nodes of \c node using DFS.
 *
 * \note \c node is automatically added to the result and returned as the first
 * node of the return set.
 * \warning this method is synchronous and recursive.
 */
std::vector<const qan::Node*>   collectDfs(const qan::Node& node, bool collectGroup = false) const noexcept;

Implementation — src/qanGraph.cpp:1978-1995:

std::vector<const qan::Node*>   Graph::collectDfs(const qan::Node& node, bool collectGroup) const noexcept
{
    std::vector<const qan::Node*> childs;
    std::unordered_set<const qan::Node*> marks;
    if (collectGroup && node.isGroup()) {
        const auto group = qobject_cast<const qan::Group*>(&node);
        if (group != nullptr) {
            for (const auto groupNode : group->get_nodes())
                collectDfsRec(qobject_cast<qan::Node*>(groupNode), marks, childs, collectGroup);
        }
    }
    for (const auto& outNode : node.get_out_nodes())
        collectDfsRec(qobject_cast<qan::Node*>(outNode), marks, childs, collectGroup);
    return childs;          // `node` never inserted into childs/marks
}

Reproduction

qan::Graph g;
auto* a = g.insertNode();   // root
auto* b = g.insertNode();
g.insertEdge(a, b);

const auto result = g.collectDfs(*a);
// Expected per docstring: result == {a, b}
// Actual:                  result == {b}

Interaction with collectSubNodes

Graph::collectSubNodes(QVector<qan::Node*> nodes, …) is implemented in terms of collectDfs(*node) (qanGraph.cpp:1997-2008). Its docstring says "nodes in node are not in returned set" — that contract holds today only because collectDfs silently drops the input node. So the two functions hold contradictory contracts about input-node membership, with the implementation matching only collectSubNodes.

Fixing collectDfs to honour its docstring will therefore change collectSubNodes behaviour as well: input nodes will start appearing in its result set, breaking its own docstring.

Suggested fix

Two coupled changes:

1. Graph::collectDfs(const qan::Node&) — insert &node into both marks and childs so the documented contract is honoured:

   marks.insert(&node);
   childs.push_back(&node);

2. Graph::collectSubNodes — after collecting per-node results, erase the input nodes to preserve its current "subnodes only" contract:

   for (const auto node : nodes)
       if (node != nullptr) r.erase(node);

Either way, the docstring and implementation should be made consistent — the current divergence is a silent footgun for callers.