End the deprecation period of using the result of a comma expression.

changelog/comma-deprecation-error.dd

@@ -0,0 +1,63 @@
+The deprecation period of using the result of comma expression has ended
+
+Comma expressions have proven to be a frequent source of confusion, and bugs.
+Using their result will now trigger an error message.
+
+$(P The comma operator (`,`) allows executing multiple expressions and
+    discards the result of them except for the last which is returned.
+
+    ---
+    int a = 1;
+    int b = 2;
+    bool ret = a == 2, b == 2; // true
+    ---
+
+    It's also common to use the comma operator in for-loop increment
+    statements to allow multiple expressions.
+
+    ---
+    for (; !a.empty && !b.empty; a.popFront, b.popFront)
+    ---
+
+    Hence, using the comma operator in for-loop increment statements is still allowed.
+)
+$(H4 Corrective Action)
+$(P If possible, split the comma operator in two statements. Otherwise use
+    lambdas.
+
+    ---
+    auto result = foo(), bar();
+
+    // split off in two statements
+    foo();
+    auto result = bar();
+
+    // or use lambdas
+    auto result = {foo(); return bar();}();
+    ---
+)
+$(H4 Rationale)
+$(P The comma operator leads to unintended behavior (see below for a selection)
+    Moreover it is not commonly used and it blocks the ability to implement tuples
+    as a language features using commas.
+
+    A selection of problems through the accidental use of the comma operator:
+    ---
+    writeln( 6, mixin("7,8"), 9 ); // 6, 8, 9
+
+    template vec(T...)(T args) { ... }
+    vec v = (0, 0, 3); // 3, because vec is variadic
+
+    int a = 0;
+    int b = 2;
+    if (a == 1, b == 2) {
+        // will always be reached
+    }
+
+    void foo(int x, int y=0) {}
+    foo((a, b)); // Oops, foo is called with x=b
+
+    synchronized (lockA, lockB) {}
+    // isn't currently implemented, but still compiles due to the comma operator
+    ---
+)

src/dmd/expressionsem.d

@@ -5844,7 +5844,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
 
         e.type = e.e2.type;
         if (e.type !is Type.tvoid && !e.allowCommaExp && !e.isGenerated)
-            e.deprecation("Using the result of a comma expression is deprecated");
+            e.error("Using the result of a comma expression is not allowed");
         result = e;
     }
 
@@ -6272,7 +6272,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             /* Rewrite to get rid of the comma from rvalue
              */
             if (!(cast(CommaExp)exp.e2).isGenerated)
-                exp.deprecation("Using the result of a comma expression is deprecated");
+                exp.error("Using the result of a comma expression is not allowed");
             Expression e0;
             exp.e2 = Expression.extractLast(exp.e2, &e0);
             Expression e = Expression.combine(e0, exp);

test/compilable/interpret3.d

@@ -1,4 +1,4 @@
-// PERMUTE_ARGS: -inline
+// PERMUTE_ARGS: -inline
 
 template compiles(int T)
 {
@@ -6288,7 +6288,7 @@ static assert(ctfe7784());
     7781
 **************************************************/
 
-static assert(({ return; }(), true));
+static assert(({ return true; }()));
 
 /**************************************************
     7785

test/fail_compilation/commaexp.d

@@ -1,13 +1,16 @@
-/* REQUIRED_ARGS: -o- -de
+/* REQUIRED_ARGS: -o-
 TEST_OUTPUT:
 ---
-fail_compilation/commaexp.d(24): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(36): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(37): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(38): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(39): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(41): Deprecation: Using the result of a comma expression is deprecated
-fail_compilation/commaexp.d(42): Deprecation: Using the result of a comma expression is deprecated
+fail_compilation/commaexp.d(27): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(39): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(40): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(41): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(42): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(44): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(45): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(56): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(69): Error: Using the result of a comma expression is not allowed
+fail_compilation/commaexp.d(81): Error: Using the result of a comma expression is not allowed
 ---
 */
 
@@ -41,3 +44,39 @@ int main () {
     ok = true, (ok = (true, false));
     return 42, 0;
 }
+
+
+/***************************************************/
+// 16022
+
+bool test16022()
+{
+    enum Type { Colon, Comma }
+    Type type;
+    return type == Type.Colon, type == Type.Comma;
+}
+
+bool test16022_structs()
+{
+    struct A
+    {
+        int i;
+        string s;
+    }
+
+    enum Type { Colon = A(0, "zero"), Comma = A(1, "one") }
+    Type type;
+    return type == Type.Colon, type == Type.Comma;
+}
+
+/********************************************/
+
+
+void bar11(int*, int*) { }
+
+void test11()
+{
+    static int* p;
+    static int i;
+    bar11((i,p), &i);
+}

test/fail_compilation/diag10862.d

@@ -57,7 +57,7 @@ fail_compilation/diag10862.d(77): Error: assignment cannot be used as a conditio
 fail_compilation/diag10862.d-mixin-80(80): Error: assignment cannot be used as a condition, perhaps `==` was meant?
 fail_compilation/diag10862.d-mixin-81(81): Error: assignment cannot be used as a condition, perhaps `==` was meant?
 fail_compilation/diag10862.d-mixin-82(82): Error: assignment cannot be used as a condition, perhaps `==` was meant?
-fail_compilation/diag10862.d-mixin-83(83): Deprecation: Using the result of a comma expression is deprecated
+fail_compilation/diag10862.d-mixin-83(83): Error: Using the result of a comma expression is not allowed
 fail_compilation/diag10862.d-mixin-83(83): Error: assignment cannot be used as a condition, perhaps `==` was meant?
 fail_compilation/diag10862.d-mixin-86(86): Error: `a + b` is not an lvalue
 fail_compilation/diag10862.d-mixin-87(87): Error: undefined identifier `c`

test/fail_compilation/fail13902.d

@@ -4,9 +4,11 @@ struct S1 { int v; }
 struct S2 { int* p; }
 class C { int v; }
 
+#line 6
 /*
 TEST_OUTPUT:
 ---
+fail_compilation/fail13902.d(45): Error: Using the result of a comma expression is not allowed
 fail_compilation/fail13902.d(32): Error: returning `& x` escapes a reference to local variable `x`
 fail_compilation/fail13902.d(33): Error: returning `&s1.v` escapes a reference to local variable `s1`
 fail_compilation/fail13902.d(38): Error: returning `& sa1` escapes a reference to local variable `sa1`
@@ -47,9 +49,11 @@ int* testEscape1()
     return null;    // ok
 }
 
+#line 49
 /*
 TEST_OUTPUT:
 ---
+fail_compilation/fail13902.d(88): Error: Using the result of a comma expression is not allowed
 fail_compilation/fail13902.d(75): Error: returning `& x` escapes a reference to parameter `x`, perhaps annotate with `return`
 fail_compilation/fail13902.d(76): Error: returning `&s1.v` escapes a reference to parameter `s1`, perhaps annotate with `return`
 fail_compilation/fail13902.d(81): Error: returning `& sa1` escapes a reference to parameter `sa1`, perhaps annotate with `return`
@@ -90,9 +94,11 @@ int* testEscape2(
     return null;    // ok
 }
 
+#line 92
 /*
 TEST_OUTPUT:
 ---
+fail_compilation/fail13902.d(123): Error: Using the result of a comma expression is not allowed
 ---
 */
 int* testEscape3(

test/fail_compilation/ice10949.d

@@ -1,7 +1,7 @@
 /*
 TEST_OUTPUT:
 ---
-fail_compilation/ice10949.d(12): Deprecation: Using the result of a comma expression is deprecated
+fail_compilation/ice10949.d(12): Error: Using the result of a comma expression is not allowed
 fail_compilation/ice10949.d(12): Error: array index 3 is out of bounds `[5, 5][0 .. 2]`
 fail_compilation/ice10949.d(12): Error: array index 17 is out of bounds `[2, 3][0 .. 2]`
 fail_compilation/ice10949.d(12):        while evaluating: `static assert((((([5, 5][3] + global - global) * global / global % global >> global & global | global) ^ global) == 9 , [2, 3][17]) || [3, 3, 3][9] is 4 && [[1, 2, 3]][4].length)`

test/runnable/aliasthis.d

@@ -551,15 +551,15 @@ void test2781()
     }
 
     eval = 0;
-    foreach (i, e; tup(tup((eval++, 10), 3.14), tup("str", [1,2])))
+    foreach (i, e; tup(tup((){eval++; return 10;}(), 3.14), tup("str", [1,2])))
     {
         static if (i == 0) assert(e == tup(10, 3.14));
         static if (i == 1) assert(e == tup("str", [1,2]));
     }
     assert(eval == 1);
 
     eval = 0;
-    foreach (i, e; tup((eval++,10), tup(3.14, tup("str", tup([1,2])))))
+    foreach (i, e; tup((){eval++; return 10;}(), tup(3.14, tup("str", tup([1,2])))))
     {
         static if (i == 0) assert(e == 10);
         static if (i == 1) assert(e == tup(3.14, tup("str", tup([1,2]))));
@@ -836,7 +836,7 @@ void test6369c()
 void test6369d()
 {
     int eval = 0;
-    Seq!(int, string) t = tup((++eval, 10), "str");
+    Seq!(int, string) t = tup((){++eval; return 10;}(), "str");
     assert(eval == 1);
     assert(t[0] == 10);
     assert(t[1] == "str");

test/runnable/constfold.d

@@ -559,7 +559,8 @@ void test13977()
     Object.init && check();
     assert(x == 0);
 
-    (check(2), false) && check();
+    check(2);
+    false && check();
     assert(x == 2); x = 0;
 }
 
@@ -588,7 +589,8 @@ void test13978()
     Object.init || check();
     assert(x == 1); x = 0;
 
-    (check(2), true) || check();
+    check(2);
+    true || check();
     assert(x == 2); x = 0;
 }
 

test/runnable/ctorpowtests.d

@@ -136,7 +136,8 @@ static assert(bazra(14)==64);
 
 void moreCommaTests()
 {
-   auto k = (containsAsm(), containsAsm());
+   (containsAsm(), containsAsm());
+   auto k = containsAsm();
    for (int i=0; i< k^^2; i+=StructWithCtor(1).n) {}
 }
 

test/runnable/lazy.d

@@ -74,7 +74,7 @@ void test1()
     );
 
     whiler( x < 100,
-        (printf("%d\n", x), x *= 2)
+        (){ printf("%d\n", x); x *= 2; }()
     );
 }
 

test/runnable/mangle.d

@@ -324,23 +324,25 @@ void test9525()
     enum result1 = "S6mangle8test9525FZ"~tl!"26"~"__T5test1S"~tl!"13"~id!("6mangle","QBc")~"5f9525Z"~id!("5test1","Qr")~"MFZ1S";
     enum result2 = "S6mangle8test9525FZ"~tl!"26"~"__T5test2S"~tl!"13"~id!("6mangle","QBc")~"5f9525Z"~id!("5test2","Qr")~"MFNaNbZ1S";
 
-    void test1(alias a)()
+    bool test1(alias a)()
     {
         static struct S {}
         static assert(S.mangleof == result1);
         S s;
         a(&s);  // Error: Cannot convert &S to const(S*) at compile time
+        return true;
     }
-    static assert((test1!f9525(), true));
+    enum evalTest1 = test1!f9525();
 
-    void test2(alias a)() pure nothrow
+    bool test2(alias a)() pure nothrow
     {
         static struct S {}
         static assert(S.mangleof == result2);
         S s;
         a(&s);  // Error: Cannot convert &S to const(S*) at compile time
+        return true;
     }
-    static assert((test2!f9525(), true));
+    enum evalTest2 = test2!f9525();
 }
 
 /******************************************/

test/runnable/sdtor.d

@@ -879,7 +879,9 @@ void test34()
     X34[2] xs;
 //  xs[0][0] = X34();
     printf("foreach\n");
-    for (int j = 0; j < xs.length; j++) { auto x = (j++,j--,xs[j]);
+    for (int j = 0; j < xs.length; j++) {
+        j++,j--;
+        auto x = xs[j];
         //foreach(x; xs) {
         //printf("foreach x.i = %d\n", x[0].i);
         //assert(x[0].i == 1);
@@ -1395,22 +1397,22 @@ void test54()
     assert(S54.t == "c");
 
     {   S54.t = null;
-        int b = 1 && (bar54(S54(1)), 1);
+        int b = 1 && (){ bar54(S54(1)); return 1;}();
     }
     assert(S54.t == "ac");
 
     {   S54.t = null;
-        int b = 0 && (bar54(S54(1)), 1);
+        int b = 0 && (){ bar54(S54(1)); return 1;}();
     }
     assert(S54.t == "");
 
     {   S54.t = null;
-        int b = 0 || (bar54(S54(1)), 1);
+        int b = 0 || (){ bar54(S54(1)); return 1;}();
     }
     assert(S54.t == "ac");
 
     {   S54.t = null;
-        int b = 1 || (bar54(S54(1)), 1);
+        int b = 1 || (){ bar54(S54(1)); return 1;}();
     }
     assert(S54.t == "");
 
@@ -1529,7 +1531,8 @@ void test57()
 
     printf("----\n");    //+
     dtor_cnt = 0;
-    if (auto s = (S57(1), S57(2), S57(10)))
+    S57(1), S57(2);
+    if (auto s = S57(10))
     {
         assert(dtor_cnt == 2);
         printf("ifbody\n");
@@ -1562,7 +1565,8 @@ void test57()
 
     printf("----\n");    //+
     dtor_cnt = 0;
-    if (auto s = (f(1), f(2), f(10)))
+    f(1), f(2);
+    if (auto s = f(10))
     {
         assert(dtor_cnt == 2);
         printf("ifbody\n");
@@ -1596,7 +1600,8 @@ void test57()
 
     printf("----\n");
     dtor_cnt = 0;
-    if ((S57(1), S57(2), S57(10)))
+    S57(1), S57(2);
+    if (S57(10))
     {
         assert(dtor_cnt == 3);
         printf("ifbody\n");
@@ -1628,7 +1633,8 @@ void test57()
 
     printf("----\n");
     dtor_cnt = 0;
-    if ((f(1), f(2), f(10)))
+    f(1), f(2);
+    if (f(10))
     {
         assert(dtor_cnt == 3);
         printf("ifbody\n");
@@ -4431,7 +4437,8 @@ struct S64
 
 S64 foo64()
 {
-    return S64((X64(), 1));
+    X64();
+    return S64(1);
 }
 
 void test64()