Compare Expressions: Fix the conditions for coalescing a node with its parent

[mgrang]

We can coalesce a BinaryNode if any one of the following conditions are
true:

1. The parent has the same operator as the current node OR
2. The current node has just one child (for example, as a result of
   constant folding) and the parent and current operators are commutative and
   associative.

Fixes issue #932

[mgrang]

The comment here needs to be updated to be in sync with the fix.

Done.

[mgrang]

Some notes from the PR review discussion:

1. Maybe we should remove the operator as part of constant folding. For example, given nodes +,1,2,3 we should not replace them with +,6 but 6. This means we need to remove the operator as part of constant folding itself. We also need to handle the case where an operator has constant and non-constant nodes. For example, given +,a,2,3 we should not get rid of the operator in this case.
   Note: Constant folding may be more broadly useful in bounds checking. So it's a good idea to make constant folding self-sufficient.

2. The tree is not really a "binary" tree but an n-ary tree. So calling the operator nodes as BinaryNode sounds confusing. Maybe we should rename BinaryNode to InnerNode or OperatorNode. This is fixed in Compare Expressions: Rename BinaryNode to OperatorNode [NFC] #946

[sulekhark]

My thoughts are as follows:
a) Should the condition 1 above not be "parent has the same operator as the current node, and the operator is commutative and associative" ?
b) If we implement constant folding to be complete in itself, then the current node can never have just one child because of constant folding. It is possible that the current node will have just one child if the operator at the current node is a unary operator.

[mgrang]

My thoughts are as follows:
a) Should the condition 1 above not be "parent has the same operator as the current node, and the operator is commutative and associative" ?
Could you elaborate why this should not be the case? We should not try to coalesce different operators and coalescing only makes sense if the operator is commutative and associative because after coalescing we also need to sort the nodes.
---> I meant the same thing. It seems like my question was confusing. The main point of my question is that the comment on lines 39 thro' 42 in the file PreorderAST.cpp is a bit unclear about what you just explained in the sentence above.

b) If we implement constant folding to be complete in itself, then the current node can never have just one child because of constant folding. It is possible that the current node will have just one child if the operator at the current node is a unary operator.
We have now implemented constant folding to be complete in itself. As part of constant folding we now invoke this method to coalesce the node. So we need this condition here. Otherwise we would end up duplicating logic to coalesce nodes.
---> Sounds good.

[sulekhark]

LGTM.