Control Flow: extend ordering

[redsun82]

In Swift we encountered a situation where ordering of nodes and edges in the control flow graph was not stable between test runs.

This patch:

• adds the successor string representation to the edge ordering
• adds an overridable getOrderDisambiguation method to both node and edge orderings

This is used in #9891

This is currently only used in tests.

[nickrolfe]

Do I understand correctly that you've observed CFG nodes that have two or more successors with the exact same locations and the same SuccessorType, but are distinguished by their toString()?

[nickrolfe]

Either way, the change looks fine, and I can see for all three languages it's only used by the CFG tests, so I don't think we need a change note.

[redsun82]

I was seeing flakiness in these lines on a branch of mine

codeql/swift/ql/test/library-tests/controlflow/graph/Cfg.expected

Lines 1514 to 1535 in 2bc6c98

	# 141| case ...
	#-----| -> =~ ...
	# 141| 0
	#-----| -> =~ ...
	# 141| =~ ...
	#-----| -> 0
	# 141| =~ ...
	#-----| no-match -> =~ ...
	#-----| match -> true
	# 141| 1
	#-----| -> =~ ...
	# 141| =~ ...
	#-----| -> 1
	# 141| =~ ...
	#-----| match -> true
	#-----| no-match -> case ...

with the successors changing order betwen runs. This change seemed to help, but now I have started seeing flakiness again, although this time it might rather be related to graph node ordering. I see an alternation between

#  141| =~ ...
#-----|  -> 0

#  141| =~ ...
#-----| no-match -> =~ ...
#-----| match -> true

#  141| 1
#-----|  -> =~ ...

#  141| =~ ...
#-----|  -> 1

#  141| =~ ...
#-----| match -> true
#-----| no-match -> case ...

and

#  141| =~ ...
#-----| no-match -> =~ ...
#-----| match -> true

#  141| =~ ...
#-----|  -> 0

#  141| 1
#-----|  -> =~ ...

#  141| =~ ...
#-----| match -> true
#-----| no-match -> case ...

#  141| =~ ...
#-----|  -> 1

I'll research this a bit more maybe before merging this

[nickrolfe]

It looks like you have multiple nodes at line 141 (with the same start/end line/column) that all have =~ ... as their toString() result. So I'm not surprised you're seeing flakiness - they are equal for all the order by expressions in the nodes predicate.

[michaelnebel]

LGTM!

[redsun82]

As the edge ordering was not enough to solve the flakiness I had, I added a further customizable string key. In #9891 I use it with getPrimaryQlClasses which seems to finally solve my flakiness issues.

[michaelnebel]

LGTM