Make comma operator return void when warnings are enabled

[schuetzm]

This is a follow-up to #3399.

As suggested by Andrei [1], the comma operator is changed to return void in -w mode. This partially fixes https://issues.dlang.org/show_bug.cgi?id=2659. Thanks go to @yebblies for his suggestions about the implementation.

[1] http://forum.dlang.org/thread/lgsel0%2429fd%241@digitalmars.com

[schuetzm]

Now Phobos fails to compile :-(

std/algorithm.d(4613): Error: cannot cast 2u to void at compile time

What's wrong with casting to void at compile time? Because the following works:

int test() {
        cast(void) 1;
        return 10;
}
enum test2 = test();

[dnadlinger]

Error: e2ir: cannot cast cast(void)1 of type void to type int indicates a problem with your implementation. Glue layer errors are never supposed to be user-visible, and break conditional compilation.

[yebblies]

Yeah, this isn't right. I'm guessing you added this to get rid of another error?

[schuetzm]

Yes, the failures in my first comment. But the e2ir error message was already there before I did this, so it's caused by something else.

[yebblies]

As you said, casting to void already works at compile time. Why is it producing an error now?

[schuetzm]

I think I figured it out. I needed to assign the type of the CastExp to the CommaExp. Let's see what the auto tester says now.

[schuetzm]

Urgh... now the type is void, but it no longer produces an error. Anyway, let's wait what Andrei has to say about the warning mode.

[schuetzm]

Ok, solved it, someone please check whether this is the correct way.

The remaining failures are caused by regular comma operator uses in std/uni.d, for which ketmar has posted a patch:
https://issues.dlang.org/show_bug.cgi?id=13419
I've opened a separate PR for them: dlang/phobos#2485

[dnadlinger]

I disagree with the idea that warnings change the language semantics, which also means that lumping this together with -w is a bad idea. Right now, there are (IIRC) a few cases where warnings as errors influence conditional compilation. We should fix those, not create even more situations where enabling/disabling warnings might silently change program behavior.

[schuetzm]

@klickverbot This was what Andrei suggested in the forum discussion. Of course, the behaviour can be changed. Maybe ask him? PING @andralex

[andralex]

The problem with this solution is it doesn't quite pinpoint the locus of the error. Consider:

auto fun(int a) {
    return ++a, a;
}

This code is legit with both int and void returns. If there's an error in the use of fun (possibly through a couple of forwardings also using auto return types), the error will be signaled at the use point, not the function itself.

Couple of possible approaches: (a) just run an analysis that makes sure the value of the comma operator is never fetched, (b) if using a type is desirable use a poisoned type such as "bottom" of which values can never be produced or copied. Also, one possibility would be to make sure the comma expression is not used to return stuff from an auto function.

[yebblies]

(a) just run an analysis that makes sure the value of the comma operator is never fetched

Something like this already exists as discardValue.

[schuetzm]

Thanks Andrei, I will have a look this evening. Could you also comment on Daniel's and David's objection about the different semantics in -w mode?

[bearophile]

@klickverbot:

I disagree with the idea that warnings change the language semantics, which also means that lumping this together with -w is a bad idea.

Is adding a -transition=commaop switch of any help?

[WalterBright]

I agree with @klickverbot that having warnings change language semantics is a very troubling idea.

In general, having compiler switches that change language semantics is a bad path to go down.

[bearophile]

WalterBright@:

I agree with @klickverbot that having warnings change language semantics is a very troubling idea.

I think it's a good idea to remove/modify from the C semantics of the comma operator. A simple solution is to go the usual route of warning/deprecation/error/reuse of the comma operator, with a short deprecation period.

[yebblies]

I think it's a good idea to remove/modify from the C semantics of the comma operator. A simple solution is to go the usual route of warning/deprecation/error/reuse of the comma operator, with a short deprecation period.

No, this would mean breaking code only to allow it again. As the plan is to just deprecate using the result of the comma operator, checking for expressions that use the result and issuing a warning should be the first step.

[bearophile]

@yebblies:

the plan is to just deprecate using the result of the comma operator

OK. Is this enough to later allow a tuple syntax?

[yebblies]

I doubt it.

[bearophile]

@yebblies: >I doubt it.

It can be just a first step. Otherwise Andrei may have to rethink about this topic/design.

[andralex]

This should go in as an opt-in error. At best we'd have a DIP for it and then opt-in via a -dip=xx option. Is anyone willing to take up on this?

[yebblies]

Warnings are opt-in. Why aren't we just issuing a warning when the result is used? Enabling warnings shouldn't change types of anything especially since you can just have warnings print without making compilation fail.
x = 1, 2;
Warning: Don't do this

[andralex]

Fine, I'm okay (for now) with a warning with no change in semantics.

[lionello]

Stumbled upon this, and +1 for issuing a warning when the result is used (so that for (;;a,b) is fine) with no change in semantics.

[munael]

(a) just run an analysis that makes sure the value of the comma operator is never fetched

Isn't this just how statements in general behave? If the comma operator stops yielding its last element then using it anywhere but where expressions can be is the same as just listing statements without return (function-wide or in a lambda).

If the comma operator returns void, how is that any different from just listing the statements with semicolons in-between? You won't be able to use the comma operator meaningfully where expressions go anyway.

What am I missing?

[ntrel]

@schuetzm Andrei recently reconfirmed his support of this change:
http://forum.dlang.org/post/nh1luh$20ds$1@digitalmars.com

Any chance of re-opening? Before did this just slip off the radar?

[schuetzm]

@ntrel As far as I remember, the implementation in this PR is incomplete. I'll leave the branch there, so anyone who wants can pick it up.