Make dominance functions consistent with definitions

[Peiffap]

Fixes #6710.
Fixes #6713.

This enforces consistency with definitions:

• The entry node does not have an immediate dominator - this follows from the definition that d immediately dominates n if d strictly dominates n + another condition, but d cannot strictly dominate itself; I've added a reference to corroborate this.
• The dominance frontier should handle loops on the entry node properly. The second linked issue (Wrong Dominance Frontiers when loop in first 2 nodes #6713) has a detailed example of what the current behavior is vs. what applying the definition leads to. It also synergizes well with the first change.

Edit: after discussing, it made more sense to exclude the start node from the returned dictionary in immediate_dominators.

[rossbar]

IIUC this implements the {start: None} pattern discussed in #6710. It seems to me that folks were converging on this as a reasonable solution there, so this PR seems in line with that discussion.

One fun little Pythonic bit with None as the value is that this overloads the meaning of None in the idom dict for the .get method, where None is the default sentinel for "key not present". However, I think this is overloading is actually okay since the whole point is start cannot have an immediate dominator. The only use-case it would screw up is users who are relying on None to indicate whether or not start is explicitly in the dict. Since the current behavior also includes start as a key (with itself as a value), this doesn't break anything in terms of backward compatibility.

My knowledge of dominating nodes is limited to a quick read of the wiki article. @Marc-Goritschnig do these changes align with your understanding of correct semantics?

[amcandio]

Why not removing start as an entry on the output? If it is not defined for start, why define an entry for it?

[Peiffap]

@rossbar You raise a valid point about overloading None that I hadn't considered!

@amcandio I believe both approaches can make sense semantically. I went with what @Marc-Goritschnig suggested because it seemed like the minimal change (and least likely to break things for users), but I agree that in principle, it seems (more?) reasonable to remove start from the returned nodes.
@dschult mentioned that users who want to identify the start node without having access to something out of the dictionary would lose information if start is excluded. Ross mentioned a use case where using None as sentinel could break things. Curious to hear what everybody thinks is least likely to break things!

[amcandio]

I'm +1 on removing it from the dictionary. Since the start is a parameter the user won't fully lose the information

[Peiffap]

The more I think about this, the more I tend to agree, Ale. I'm going to wait until some more opinions are shared but I'd say I'm leaning more towards excluding start from the returned dictionary.

[dschult]

The last comment in #6710 points out that not all the nodes are present in the networkx result for immediate_dominators. So having all nodes in a returned dict is not as relevant for this literature as for some other algorithms.

I'm good for excluding start from the returns dict.

[Peiffap]

I've amended the previous changes so immediate_dominators now no longer includes the start node.

[rossbar]

I finally had a chance to go over this after the last round of discussion re: what to do about start. Everything LGTM, the behavior of both immediate_dominators and dominance_frontiers is consistent with the expectations from the linked issues. I have no opinion about whether start should be included in immediate_dominators, so that seems fine to me too. Thanks @Peiffap @amcandio and @dschult for working through this!

[Peiffap]

@dschult I'm not sure how you feel about "friendly" reminders (please tell me if this is not okay), but if you have some time, I think this PR would be nice to get into the next release.

The PR description is up-to-date with the latest changes, in case you want to rely on that for an overview. Let me know if there's anything else I can do here to make reviewing this easier! :)

[dschult]

Gentle reminders is good for me. I actually started looking at this last night. So the timing is good. :)

[LebedevRI]

@Peiffap @dschult thank you!