Deprecate support for the comma operator outside of for() conditions and increments

[schuetzm]

See issue 2659 [1]. The comma operator is a source of hard-to-detect bugs, is mostly unnecessary, and blocks the path to a nice template literal syntax. The only place where it makes sense to keep it is in the condition and increment parts of for statements.

Is it better to keep the functionality and deprecate it first?

(This PR depends on dlang/phobos#2041 for Phobos to compile again.)

[1] https://d.puremagic.com/issues/show_bug.cgi?id=2659

[yebblies]

Should be a warning first.

[schuetzm]

Ok, changed it into a deprecation warning.

[ghost]

Needs a test-case and an ok from @WalterBright and @andralex.

[yebblies]

I meant an actual warning, not a deprecation warning. The progression is usually valid -> warning -> deprecation -> error -> fails to parse.

[schuetzm]

Changed it to a normal warning and added test-cases for for and otherwise.

[andralex]

So this found a bug, which is nice. It has, however, broken druntime and phobos and required code changes that for the most part are not better, not worse, just different. It is likely to break other projects in similar ways and to require changes that are little else but churn.

Are we sure this is on the short list of breaking changes that are so valuable, and improve code quality so much, that they're worth it?

[yebblies]

Are we sure this is on the short list of breaking changes that are so valuable, and improve code quality so much, that they're worth it?

Oooh yes.

[dnadlinger]

Are we sure this is on the short list of breaking changes that are so valuable, and improve code quality so much, that they're worth it?

Oooh yes.

Oh yes indeed. In my eyes, D's comma operator is one of the most striking anachronisms in what otherwise is a modern language.

[deadalnix]

Yes, this has been discussed to death several times. This must go. Taking the deprecation path seems like the way to go.

[etcimon]

I'm totally against this. I love the comma operator, it allows for simple one-liners like:

while(!r.empty) tmp ~= cast(string) r.front, r.popFront();
or
if (!canAccess(key)) return setEC(StatusCode.Undefined), string[].init;
or this would require 4 braces:
foreach (field; fields) if ( (field in data) !is null ) data[field].free(), data.remove(field), cnt++;
or this makes it obvious to me what's being used in the while statement:
while(el = cast(string) data[pos], tot < count && pos < len){

Take this for example: https://github.com/globecsys/cache.d/blob/master/chd/table.d#L367
if (el == value) el.free(), data.linearRemove( data[pos..(pos+1)] ), tot++, len--;

It would explode to 5 lines at least. The code becomes completely full of braces and vertical. You may have your own opinions on readability, and I agree I don't have a widespread way of coding, but I don't think D should restrict itself to one possible aesthetic arrangement. And, I take a lot of pleasure in writing wider statements, it fits very well into the screen.

[yebblies]

while(!r.empty) { tmp ~= cast(string) r.front; r.popFront(); }

if (!canAccess(key)) { setEC(StatusCode.Undefined); return string[].init; }

foreach (field; fields) if ( (field in data) !is null ) { data[field].free(); data.remove(field); cnt++; }

while(() { el = cast(string) data[pos]; return tot < count && pos < len; }()){

[andralex]

I have to say I also like a lot of the "before" code in druntime and phobos.

[etcimon]

The workarounds are correct though to me it feels "wrong" with more braces {try fitting a joke about lisp here} but then I feel I'm finding myself to be in a subjective discussion that has been discussed too much for me to have a say, so I'll leave it at that.

[bearophile]

@etcimon: if I find code like yours, I systematically remove all the commas, to make the code safer and cleaner. Let's move from such ancient problems, D has far more complex problems to face than the desire to pack as much as possible in oneliners.
@yebbles: your last line with a while is still in need to be refactored.

[etcimon]

D has far more complex problems to face than the desire to pack as much as possible in oneliners.

If D has more complex problems, why try to fix what isn't broken based on the premise of the age of an operator? (btw, all operators date from approx the 60s)

[deadalnix]

That is ridiculous. This has been debated to death. @andralex you even made a DIP about it (DIP19) in 2012 (!). You'll find discussion about this that goes back to at least 2009. We are not going anywhere by moving forward and backward continuously, we are simply accumulating technical debt.

For the record:
http://forum.dlang.org/thread/k3ns2a$1ndc$1@digitalmars.com

@etcimon It isn't simply about fixing what is not broken, but also about making room for tuples.

[andralex]

What would be great would be if people worked on marking real progress for D instead of on changing what works for tweaks that are debatable at worst, and modest at the very best. We have so many big rocks to move yet here we are again getting back to a little tweak.

[etcimon]

@deadalnix I wasn't around back then and this seemed like a surprise. I read quickly and this would make room for multiple value return like ruby or a simpler boost::tie?

[yebblies]

What would be great would be if people worked on marking real progress for D instead of on changing what works for tweaks that are debatable at worst, and modest at the very best. We have so many big rocks to move yet here we are again getting back to a little tweak.

So you think new contributors should start with the big rocks?

[andralex]

At best they'd start with bug fixes and small additions. Breaking changes with little upside are mostly churn, churn, churn.

[yebblies]

Breaking changes with little upside

Preventing hard-to-find bugs, improving readability? Don't most C/C++/everything style guides say not to use the comma operator outside of for?

[etcimon]

Preventing hard-to-find bugs, improving readability? Don't most C/C++/everything style guides say not to use the comma operator outside of for?

According to this generally accepted explanation, this pull request officially removes "expression programming" from D in favor of "functor-based programming", and thus the ternary ?: operator should also be deprecated.

[andralex]

Don't most C/C++/everything style guides say not to use the comma operator outside of for?

Not the one I wrote.

[yebblies]

Not the one I wrote.

Touché.

@etcimon

According to this generally accepted explanation, this pull request officially removes "expression programming" from D in favor of "functor-based programming", and thus the ternary ?: operator should also be deprecated.

Maybe, but that's not the motivation. The comma operator's major problem is that it looks exactly like the argument separator, but has different semantics.

[etcimon]

Maybe, but that's not the motivation. The comma operator's major problem is that it looks exactly like the argument separator, but has different semantics.

Every letter "e" in this sentence are exactly the same but are part of different words thus have different semantics...

[yebblies]

Every letter "e" in this sentence are exactly the same but are part of different words thus have different semantics...

Do you really not get it or are you just wasting my energy?

[bearophile]

Every letter "e" in this sentence are exactly the same but are part of different words thus have different semantics...

A good language should try to make different semantics look different, to reduce the probability of mistakes. See an example here: http://www.viva64.com/en/b/0240/ , from the CryEngine 3 SDK, look for "A parenthesis in a wrong place". The line of code:

else if(pMgr->ShouldRecordEvent(eSE_Weapon), pOwnerRaw)

Here the problem is the parentheses placement, the correct code is:

else if(pMgr->ShouldRecordEvent(eSE_Weapon, pOwnerRaw))

The wrong code is impossible if we disallow those tricky usages of the comma operator.

[bearophile]

@etcimon:

If D has more complex problems, why try to fix what isn't broken based on the premise of the age of an operator? (btw, all operators date from approx the 60s)

I think the comma is broken. Modern languages should not have a bug prone operator that is also not intuitive. C# designers have made the right design decisions of not putting the comma operator, of having final fields on default, etc. How much people do you know that is asking vehemently for the introduction of comma operator in C#? (Yes, I know that removing an operator is not the same thing as deciding to not add it from the start, but there's still time to influence Andrei's mind and fix this D design mistake).

[etcimon]

Do you really not get it or are you just wasting my energy?

It made no sense to me why would we possible disallow two different semantics for a character.

The wrong code is impossible if we disallow those tricky usages of the comma operator.

That makes sense, reminds me of the iphone ssl double-goto indent bug but the only fix was to indent the code correctly. It's not bug-prone, it's just highly unsafe/insecure because the compiler can't even be taught to warn you on that one, same for the wrongly indented goto. Should we force indentation?

[bearophile]

but the only fix was to indent the code correctly.

If you compile your D code with warnings, the D compiler gives you a "code not reachable" warning on code like the iPhone double-goto, that avoids you that bug.

It's not bug-prone, it's just highly unsafe/insecure because the compiler can't even be taught to warn you on that one,<

"highly unsafe/insecure" code is bad and we should try to avoid it.

Should we force indentation?

This is off-topic in this thread because we are not discussing about indentation. But you know there are languages that indeed force indentation. Python is like this, and it's one of the ten most used languages in the world, and a high percentage of its users are happy with the language. Other languages that partially require indentations are F#, Haskell, etc.

Go language essentially forces indentation because several repositories force the usage of the standard code formatting tool.

And even for C there are lints that warn your usages of suspicious indents that go against the code semantics.

[etcimon]

If I wanted to have one statement per line or forced indentations, I'd have chosen another language. Right now I have flexibility and I'm saddened to lose it, but I have high hopes that it'll be compensated with more features than making the language more idiot-proof and empty.

[bearophile]

@etcimon:

than making the language more idiot-proof

All engineers worth such name know how important is to make things idiot-proof. And the older you get, the more you value that.

[quickfur]

I vote for killing the comma operator. It should be special-cased syntax in for-loops, not a language-wide "operator" of questionable semantics. Don't get me wrong -- I'm a C/C++ coder and I do enjoy the occasional clever one-liner using the comma operator -- but from a longer-term POV, this hurts code readability, maintainability, and writability, and is thus a net minus that should be fixed while we still can.

[andralex]

@etcimon there's a compromise being discussed in the forum

[etcimon]

All engineers worth such name know how important is to make things idiot-proof. And the older you get, the more you value that.

“A common mistake that people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools.”

T o(T, U...)(T t, U u){ return t; }
if (something) return o(1+1, call(), some(&fct, 1+1), i++);

This usage of variadic functions is an example of the "ingenuity of complete fools"..

[bearophile]

It's a mistake to try to design something "completely foolproof", you hit diminishing returns. What you do is to try to avoid the most dangerous cases first (crashing planes, blowing up plants, corners that cut fingers, breaking wheels on slippery asphalt during a break, etc), and you try to remove the most common mistakes, possibly using real-world usage data.

[schuetzm]

Wow, seems I accidentally stumbled upon a hot topic here. Maybe I should have added more explanation...

My main motivation is to make room for a better tuple literal syntax. I've been lurking in the forums for quite some time, and I followed the aforementioned DIP19 debate too. My impression was that people mostly agreed to deprecate the comma operator (even though there was no consensus about the actual tuple syntax), and since then, people kept asking why the deprecation wasn't done yet. Therefore, I'm surprised that this change should turn out so controversial...

@etcimon I would even agree with your stance that the possible bugs by themselves don't warrant changing the language. It's what we can gain in exchange (tuple literals) that tips the balance, IMO.

[DIP19] http://forum.dlang.org/thread/k3ns2a$1ndc$1@digitalmars.com

[schuetzm]

And I'd like to add that there were already other PRs removing commas from druntime and Phobos, which I understood to be preparatory work for deprecating the comma operator.

[andralex]

@schuetzm: many of us would agree to make the comma more powerful. The compromise discussed in the forum (no change to the use except the result cannot be used) seems a good pathway toward that state, and @WalterBright and myself would back it if implemented as a -w warning. Please let me know if you'd like to refactor this pull request into that.

[MartinNowak]

An argument against the comma operator, it cannot be used with lambdas.
An argument for the comma operator, it's sometimes handy for one-line returns.

if (args.length < 2)
    return print_usage(), EXIT_FAILURE;

if (args.length < 2)
{
    print_usage();
    return EXIT_FAILURE;
}

[dnadlinger]

@MartinNowak: if (args.length < 2) { print_usage(); return EXIT_FAILURE; }. Much clearer imho, as it doesn't suggest that the code somehow "returns print_usage()".

[etcimon]

I've thought about it, and there's some really nice ways D could replace the comma.

if (args.length < 2) return EXIT_FAILURE.and( print_usage() );

where and is defined by: T and(T, U...)(T t, U u){ return t; }

Some other name ideas: also, compute, eval

[andralex]

@etcimon please not...

[yebblies]

I've thought about it, and there's some really nice ways D could replace the comma.

Or you know, stay sane and just use braces. { print_usage(); return EXIT_FAILURE; }.

[schuetzm]

@andralex I will try to refactor it in the next few days.

[Safety0ff]

I've thought about it, and there's some really nice ways D could replace the comma.

How about:
void then(T...)(T t) {}
For cases such as:

if (condition)
then (a = 1, b = 2);

[MartinNowak]

@MartinNowak: if (args.length < 2) { print_usage(); return EXIT_FAILURE; }. Much clearer imho, as it doesn't suggest that the code somehow "returns print_usage()".

Only that every formatting tool would turn this into a 5-liner. Anyhow I wouldn't mind if it vanished and I don't want to add more fuel to an endless debate.

[andralex]

fantastic, thanks!

[yebblies]

Only that every formatting tool would turn this into a 5-liner.

For good reason...

[schuetzm]

I started implementing the alternative solution, i.e. make the comma operator return void when -w is used. Here is the current status:
https://github.com/schuetzm/dmd/tree/comma-operator-void
https://github.com/schuetzm/phobos/tree/comma-operator-void

Unfortunately it doesn't really work yet. In general, it correctly changes the type of the CommaExp to void, but there must be something missing:

int test2659()
{
    return (0, 1);
}

When compiled with dmd -w ..., this asserts:
dmd: expression.c:2705: virtual dinteger_t IntegerExp::toInteger(): Assertion 0' failed.`

The following compiles and prints "x = 1" for both calls, although it shouldn't:

import std.stdio;

void a(int x) {
    writeln("x = ", x);
}

void b(int x) {
    writeln("x = ", x);
}

void b() {
    writeln("x = void");
}

void main() {
    a((0, 1));
    b((0, 1));
}

So, the type returned by CommaExp is partially ignored for implicit conversions (?) and, it seems, completely when passing arguments and for overload resolution.
I don't know DMD's internals well enough to understand what's happening. I would appreciate if someone more experienced with the compiler than me could give me some hints.

[yebblies]

You should really ping when a while goes by with no responses. Nobody is (usually) looking through for pull requests with unanswered questions.

A few points:

• There is no need to introduce multiple subclasses of CommaExp. A flag in CommaExp marking it as internal should be sufficient.
• I don't really like -w changing the type of CommaExp. I would prefer it just warning if the result is being used.
• To change the type of an expression to void, you should wrap it in a cast(void)(e) expression and return that. Simply making the type Tvoid is dodgy, because the type is supposed to be the type of e2.

[schuetzm]

@yebblies Thanks for your comments.

• Almost all uses of CommaExp come from lowering. I decided to create separate classes with different names instead of introducing a flag defaulting to false, to make it a compile error when someone who is not aware of the change tries to use CommaExp inadvertantly.
• About -w: This was what Andrei suggested in the forum discussion: http://forum.dlang.org/thread/lgsel0$29fd$1@digitalmars.com
• I'll change it to use the suggested casting technique instead.

[schuetzm]

Closing this PR, follow-up is here: #3943

[yebblies]

Almost all uses of CommaExp come from lowering. I decided to create separate classes with different names instead of introducing a flag defaulting to false, to make it a compile error when someone who is not aware of the change tries to use CommaExp inadvertantly.

Yes, I figures this was why, but it results in a LOT more changes than would otherwise be necessary. It's just not worth it for CommaExp.

About -w: This was what Andrei suggested in the forum discussion:

I know. But it's not nice to have enabling warnings change the types of expressions. This means that enabling -w will cause errors.