logic: fix multi-level inheritance edges when there are diamonds (#1142)

The old implementation produces several multi-inheritance edges between
two classes if there are multiple inheritance paths between them due to
multiple inheritance diamonds. The new implementation produces only one
edge in such a case.

Also, the old implementation has performance issues when there is a huge
number of inheritance paths starting at the focused class. With a
pathological multiple inheritance class structure, the number of paths
can be exponential in the number of involved classes. The new
implementation solves this issue by considering subgraphs instead of
paths.

The following is a small problematic example, which does not show the
performance problem due to its small size. However, when focusing on
X::foo, the old implementation produces 4 different multi-level
inheritance edges from X to B1 while the new implementation produces
only one multi-level inheritance edge, which is basically the union of
the edges of the old implementation:

    struct B1 {
    	virtual void foo() = 0;
    };
    struct C1 : B1 {};
    struct D1 : B1 {};
    struct B2 : C1, D1 {};
    struct C2 : B2 {};
    struct D2 : B2 {};
    struct B3 : C2, D2 {};

    struct X : B3 {
    	void foo() override {};
    };

    int main () {
    	X x;
    	x.foo();
    	return 0;
    }
This commit is contained in:
Toni Dietze
2021-02-23 23:04:18 +01:00
committed by GitHub
parent 4b922ed103
commit 7d344fb08e
2 changed files with 126 additions and 32 deletions
+90 -25
View File
@@ -114,31 +114,27 @@ void HierarchyCache::HierarchyNode::setIsImplicit(bool isImplicit)
m_isImplicit = isImplicit;
}
void HierarchyCache::HierarchyNode::addInheritanceEdgesRecursive(
Id startId,
const std::set<Id>& inheritanceEdgeIds,
const std::set<Id>& nodeIds,
std::vector<std::tuple<Id, Id, std::vector<Id>>>* inheritanceEdges)
std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>
HierarchyCache::HierarchyNode::getReverseReachableInheritanceSubgraph() const
{
for (size_t i = 0; i < m_bases.size(); i++)
std::map<Id, std::vector<std::pair<Id, Id>>> reverseGraph;
reverseGraph.try_emplace(getNodeId()); // mark start node as visited
getReverseReachableInheritanceSubgraphHelper(reverseGraph);
return reverseGraph;
}
void HierarchyCache::HierarchyNode::getReverseReachableInheritanceSubgraphHelper(
std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>& reverseGraph) const
{
for (size_t i = 0; i < m_bases.size(); ++i)
{
if (inheritanceEdgeIds.find(m_baseEdgeIds[i]) != inheritanceEdgeIds.end())
{
continue;
}
HierarchyNode* base = m_bases[i];
Id baseId = base->getNodeId();
std::set<Id> inheritanceEdgeIds2 = inheritanceEdgeIds;
inheritanceEdgeIds2.insert(m_baseEdgeIds[i]);
if (nodeIds.find(baseId) != nodeIds.end())
auto emplacedBase = reverseGraph.try_emplace(base->getNodeId());
emplacedBase.first->second.push_back({getNodeId(), m_baseEdgeIds[i]});
if (emplacedBase.second)
{
inheritanceEdges->push_back({startId, baseId, utility::toVector(inheritanceEdgeIds2)});
base->getReverseReachableInheritanceSubgraphHelper(reverseGraph);
}
base->addInheritanceEdgesRecursive(startId, inheritanceEdgeIds2, nodeIds, inheritanceEdges);
}
}
@@ -340,20 +336,89 @@ bool HierarchyCache::nodeIsImplicit(Id nodeId) const
return false;
}
std::vector<std::tuple<Id, Id, std::vector<Id>>> HierarchyCache::getInheritanceEdgesForNodeId(
Id nodeId, const std::set<Id>& nodeIds) const
std::vector<std::tuple</*source*/ Id, /*target*/ Id, std::vector</*edge*/ Id>>>
HierarchyCache::getInheritanceEdgesForNodeId(
Id sourceId, const std::set<Id>& targetIds) const
{
// For two nodes s and t of a graph g0, this function determines the subgraph g2 that consists
// of all nodes and edges that are reachable by going from s forwards and from t backwards as
// follows: First the subgraph g1 that consists of all nodes and edges that are reachable from s
// is determined. Afterwards g2 is determined by keeping only those nodes and edges of g1 that
// are reachable by going from t backwards. If t is not in g1, then the g2 is empty.
//
// For example (edges are pointing upwards):
//
// g0 * g1 * g2
// | |
// t t t
// | | |
// * * * * *
// \ / \ \ / \ / \
// * * * * * *
// \ / \ \ / \ /
// * * * *
// | | |
// s s s
// |
// *
std::vector<std::tuple<Id, Id, std::vector<Id>>> inheritanceEdges;
HierarchyNode* node = getNode(nodeId);
if (node)
if (targetIds.empty())
{
node->addInheritanceEdgesRecursive(node->getNodeId(), {}, nodeIds, &inheritanceEdges);
return inheritanceEdges;
}
HierarchyNode* sourceNode = getNode(sourceId);
if (!sourceNode)
{
return inheritanceEdges;
}
std::map<Id, std::vector<std::pair<Id, Id>>> reverseGraph
= sourceNode->getReverseReachableInheritanceSubgraph();
for (Id targetId : targetIds)
{
std::set<Id> nodes;
std::vector<Id> edges;
getReverseReachable(targetId, reverseGraph, nodes, edges);
if (!edges.empty())
{
inheritanceEdges.push_back({sourceId, targetId, std::move(edges)});
}
}
return inheritanceEdges;
}
void HierarchyCache::getReverseReachable(
Id nodeId,
const std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>& reverseGraph,
std::set<Id>& nodes,
std::vector<Id>& edges)
{
if (!nodes.insert(nodeId).second)
{
return;
}
auto search = reverseGraph.find(nodeId);
if (search == reverseGraph.end())
{
return;
}
for (const std::pair<Id, Id>& nodeAndEdge : search->second)
{
Id node = nodeAndEdge.first;
Id edge = nodeAndEdge.second;
edges.push_back(edge);
getReverseReachable(node, reverseGraph, nodes, edges);
}
}
HierarchyCache::HierarchyNode* HierarchyCache::getNode(Id nodeId) const
{
auto it = m_nodes.find(nodeId);
+36 -7
View File
@@ -33,10 +33,29 @@ public:
bool nodeIsVisible(Id nodeId) const;
bool nodeIsImplicit(Id nodeId) const;
std::vector<std::tuple<Id, Id, std::vector<Id>>> getInheritanceEdgesForNodeId(
Id nodeId, const std::set<Id>& nodeIds) const;
std::vector<std::tuple</*source*/ Id, /*target*/ Id, std::vector</*edge*/ Id>>>
getInheritanceEdgesForNodeId(Id sourceId, const std::set<Id>& targetIds) const;
private:
/**
* Determine nodes and edges from which a specific node can be reached in a reversed graph.
*
* A reversed graph can be produced by HierarchyNode::getReverseReachableInheritanceSubgraph().
*
* @param[in] nodeId ID of the target node.
* @param[in] reverseGraph The reversed graph.
* @param[out] nodes The nodes from which the node @p nodeId can be reached.
* @param[out] edges The edges from which the node @p nodeId can be reached.
*
* @pre The arguments for @p nodes and @p edges must be provided empty.
*/
static void getReverseReachable(
Id nodeId,
const std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>&
reverseGraph,
std::set<Id>& nodes,
std::vector<Id>& edges);
class HierarchyNode
{
public:
@@ -67,13 +86,23 @@ private:
bool isImplicit() const;
void setIsImplicit(bool isImplicit);
void addInheritanceEdgesRecursive(
Id startId,
const std::set<Id>& inheritanceEdgeIds,
const std::set<Id>& nodeIds,
std::vector<std::tuple<Id, Id, std::vector<Id>>>* inheritanceEdges);
/**
* Determine the reversed subgraph of all nodes and edges that are reachable from this node.
*
* The subgraph is represented by a map that maps a node ID *t* to a set of pairs where each
* pair consists of a node ID *s* and and edge ID *e* such that *e* refers to an edge from
* *s* to *t*. Note that the mapping is reversed compared to the edges.
*/
std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>
getReverseReachableInheritanceSubgraph() const;
private:
/**
* Helper for getReverseReachableInheritanceSubgraph().
*/
void getReverseReachableInheritanceSubgraphHelper(
std::map</*target*/ Id, std::vector<std::pair</*source*/ Id, /*edge*/ Id>>>&) const;
const Id m_nodeId;
Id m_edgeId;