Remove usage of CommaExp

[mathias-lang-sociomantic]

This removes all usage of the comma expression in D runtime.
One of the commit is a bug fix, the second one should be no-ops.
The only useful pattern I could find for CommaExp is within for loops, however the cost/benefit ratio is too damn low to justify it.
Ideally we should have a way to do it in a more D-ish maner, e.g. for (; i < j; { i++; j-- }) however this isn't currently inlined in -O -inline by DMD, so I do not want to introduce such a syntax ATM.

About the changes: If the condition is dependent on both the post-loop operations, I moved them together to make it more readable (see the for -> while change for example).

[ibuclaw]

Bugzilla reference for bugfix?

[ibuclaw]

Unittests for bug fix?

[Geod24]

Updated with an issue number. But unittesting a constant doesn't really makes sense.

[schveiguy]

As much as I want to remove general comma operator use support, I don't think we will ever get acceptance from @WalterBright on removing it inside for loop increments. I'd say leave those. I could be wrong, but the rewrites for those for loops are... awful. The others I agree with completely.

[wilzbach]

@mathias-lang-sociomantic how about enabling dscanner in the build setup?
It allows to check for the use of the comma operator (and a lot of other cool stuff), we use it at Phobos and it's not that hard to setup ;-)

[schveiguy]

Note: if we were to overtake comma operator for tuple use, We can go in stages:

1. Deprecate/remove general comma operator use. Leave for increments as allowed
2. Add tuple support, still allow comma operator in for loop increment, but treat whole expression as a tuple.
3. (optional) remove support for comma operator in for-loop, require using specific tuple syntax (.e.g. for(;;(i++, j++)) )

[leandro-lucarella-sociomantic]

I recommend splitting the PR in two, one for the fixed issue (which seems uncontroversial) and the comma thing (that seems a bit more controversial).

[Geod24]

As much as I want to remove general comma operator use support, I don't think we will ever get acceptance from @WalterBright on removing it inside for loop increments. I'd say leave those. I could be wrong, but the rewrites for those for loops are... awful. The others I agree with completely.

The usage in for loop is the only valid usage I could find through the code base so far (and luckily the most common one). I wouldn't call the rewrite awful, except for the requirement to duplicate statement when continue is used.

There is more usage of the comma operator in for loop than just increment. I found some quite common for(; !a.empty; a.popFront, b.popFront). Some assignments as well.
Ultimately, I'd like the {} syntax to not introduce so much overhead.

@mathias-lang-sociomantic how about enabling dscanner in the build setup?

Feel free to do it, but that's not the point of this P.R.

Note: if we were to overtake comma operator for tuple use, We can go in stages:

Good points. I had a wrong memory about the proposed tuple syntax, and thought it would make the grammar ambiguous. I'll rework the DMD code to allow it in for loops.

I recommend splitting the PR in two, one for the fixed issue (which seems uncontroversial) and the comma thing (that seems a bit more controversial).

Will do.

[schveiguy]

There is more usage of the comma operator in for loop than just increment.

By increment, I just meant the increment (3rd) expression of the for statement.

[Geod24]

Updated

[schveiguy]

OK, this looks good I think. Even if we don't remove comma operators from the language in some capacity, these changes are very minor and I think the new code is clearer.

[DmitryOlshansky]

LGTM

[DmitryOlshansky]

brace on its own line

[MartinNowak]

brace on its own line

done 8e09c99

[Geod24]

Thanks