Preserve the order of sons when splitting ternary node

[Troublor]

Fix #1846

The order of sons of node matters.
Node with NodeType.IF relies on the order of the sons to determine which son is the true branch and which is the false branch.

However, the _split_ternary_node function during AST parsing does not preserve the son order of the father node.

slither/slither/solc_parsing/declarations/function.py

Lines 1397 to 1400 in 0ec4874

	for father in node.fathers:
	father.remove_son(node)
	father.add_son(condition_node.underlying_node)
	condition_node.underlying_node.add_father(father)

When the father node is a node with multiple sons, and the index of the son to remove is not -1, the order of sones in the father node will be changed after remove_son and add_son.
As a result, the issue in #1846 occurs.

This PR adds a new utility method, replace_son, in Node class to preserve the order of sones when replacing a son.
So that #1846 can be fixed.

Two bug-revealing test cases are also added.

[Troublor]

The parser tests fail due to this test case in ast-parsing test:

slither/tests/ast-parsing/conditional-all.sol

Line 5 in 0ec4874

1 + 2 == 3 ? 4 + 5 == 6 ? int8(0) : -1 : -2;

It seems that the expected CFG is wrong due to the same issue #1846.
@0xalpharush Could you spare some time to look into the test case and confirm that the expected CFG is wrong?

[0xalpharush]

@Troublor I haven't had a chance to investigate this, but your change likely requires updating some of the AST parsing tests which generate CFGs (they're not necessarily correct). If you would make your changes against dev, that would make this easier to merge in the future

[Troublor]

@Troublor I haven't had a chance to investigate this, but your change likely requires updating some of the AST parsing tests which generate CFGs (they're not necessarily correct). If you would make your changes against dev, that would make this easier to merge in the future

@0xalpharush I have changed the base branch to dev. But the tests still fail due to the same reason (incorrect expected CFGs). Am I supposed to update the expected CFG in ast parsing tests myself?

[0xalpharush]

You can delete the old artifacts for the failing : rm tests/e2e/solc_parsing/test_data/expected/conditional-all*

Then to regenerate the artifacts:

make dev
source ./env/bin/activate
python tests/e2e/solc_parsing/test_ast_parsing.py --generate

Then, add the updated artifacts to git.

At first glance, I think the update is right. The current CFG seems to incorrect point 14 - True -> 16, 14 - False -> 26 and that's correctly reversed in the new CFG.

[Troublor]

You can delete the old artifacts for the failing : rm tests/e2e/solc_parsing/test_data/expected/conditional-all*

Then to regenerate the artifacts:

make dev
source ./env/bin/activate
python tests/e2e/solc_parsing/test_ast_parsing.py --generate

Then, add the updated artifacts to git.

At first glance, I think the update is right. The current CFG seems to incorrect point 14 - True -> 16, 14 - False -> 26 and that's correctly reversed in the new CFG.

Thanks for the guideline. I have updated the test artifacts, and the tests seem to be all passing.

[montyly]

Great catch and fix @Troublor. Nice addition to tests/unit/slithir/test_ssa_generation.py too. Thanks for the contribution