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

wilzbach · wilzbach · commit 9eccdc443e86 · 2018-01-31T06:30:44.000+01:00
diff --git a/changelog/comma-deprecation-error.dd b/changelog/comma-deprecation-error.dd
@@ -0,0 +1,4 @@
+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.
diff --git a/src/dmd/expressionsem.d b/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("The result of a comma expression can't be used");
         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("The result of a comma expression can't be used");
             Expression e0;
             exp.e2 = Expression.extractLast(exp.e2, &e0);
             Expression e = Expression.combine(e0, exp);
diff --git a/test/compilable/interpret3.d b/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
diff --git a/test/fail_compilation/commaexp.d b/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: The result of a comma expression can't be used
+fail_compilation/commaexp.d(39): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(40): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(41): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(42): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(44): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(45): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(56): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(69): Error: The result of a comma expression can't be used
+fail_compilation/commaexp.d(81): Error: The result of a comma expression can't be used
 ---
 */
 
@@ -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);
+}
diff --git a/test/fail_compilation/diag10862.d b/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: The result of a comma expression can't be used
 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`
diff --git a/test/fail_compilation/fail13902.d b/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: The result of a comma expression can't be used
 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: The result of a comma expression can't be used
 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: The result of a comma expression can't be used
 ---
 */
 int* testEscape3(
diff --git a/test/fail_compilation/ice10949.d b/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: The result of a comma expression can't be used
 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)`
diff --git a/test/runnable/aliasthis.d b/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");
diff --git a/test/runnable/constfold.d b/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;
 }
 
diff --git a/test/runnable/ctorpowtests.d b/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) {}
 }
 
diff --git a/test/runnable/lazy.d b/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; }()
     );
 }
 
diff --git a/test/runnable/mangle.d b/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();
 }
 
 /******************************************/
diff --git a/test/runnable/sdtor.d b/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()
diff --git a/test/runnable/test16115.d b/test/runnable/test16115.d
@@ -25,7 +25,7 @@ auto call()
     else // assert error
     {
         //return n = tagx, null;
-        return n = Test.tag, null;
+        return n = Test.tag;
         //return n = Test.tag;
     }
 }
diff --git a/test/runnable/test22.d b/test/runnable/test22.d
@@ -201,7 +201,9 @@ class B9
     {
         A9 test1 = new A9(1, 2, 3);
         A9 test2 = new A9(1, 2, 3, 4);
-        int[3] arg; A9 test3 = new A9((arg[0]=1, arg[1]=2, arg[2]=3, arg));
+        int[3] arg;
+        arg[0]=1, arg[1]=2, arg[2]=3;
+        A9 test3 = new A9(arg);
     }
 }
 
diff --git a/test/runnable/test42.d b/test/runnable/test42.d
@@ -3506,7 +3506,7 @@ void test215()
     enum assocarrayliteral = Q!( [1:2] ).q.stringof;
     enum complex80 = Q!( 1+1.0i ).q.stringof;
     //enum dottype = Q!( C.Object.toString ).q.stringof;
-    enum halt = (assert(0), 0).stringof;    // ICE w/ -release
+    enum halt = 0.stringof;    // ICE w/ -release
     //enum remove = Q!( [1:2].remove(1) ).q.stringof;
     enum templat = Q!( Q ).q.stringof;
 }
diff --git a/test/runnable/testaa2.d b/test/runnable/testaa2.d
@@ -150,7 +150,7 @@ void test3825()
      * it has no side effect. Then optimizer eliminate it completely, and
      * whole expression succeed to run in runtime. */
     int n = 0;
-    assert(thrown(aax[(n=aax[1], 0)] = 0)); // accessing aax[1] in key part, throws OK
+    assert(thrown(aax[((){ n=aax[1]; return 0;}())] = 0)); // accessing aax[1] in key part, throws OK
 
     // This works as expected.
     int[int][int] aaa;
diff --git a/test/runnable/testscope.d b/test/runnable/testscope.d
@@ -293,8 +293,6 @@ void test11()
     static int* p;
     static int i;
     bar11(p, &i);
-
-    bar11((i,p), &i);  // comma expressions are deprecated, but need to test them
 }
 
 /********************************************/
diff --git a/test/runnable/untag.d b/test/runnable/untag.d
@@ -22,7 +22,7 @@ bool startsWithConsume(alias pred = "a == b", R1, R2)(ref R1 r1, R2 r2)
         r.popFront();
         r2.popFront();
     }
-    return r2.empty ? (r1 = r, true) : false;
+    return r2.empty ? (){ r1 = r; return true;}() : false;
 }
 
 
diff --git a/test/runnable/xtest46.d b/test/runnable/xtest46.d

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// PERMUTE_ARGS: -inline`
	`1`	`+// PERMUTE_ARGS: -inline`
`2`	`2`
`3`	`3`	`template compiles(int T)`
`4`	`4`	`{`
`@@ -6288,7 +6288,7 @@ static assert(ctfe7784());`
`6288`	`6288`	`7781`
`6289`	`6289`	`**************************************************/`
`6290`	`6290`
`6291`		`-static assert(({ return; }(), true));`
	`6291`	`+static assert(({ return true; }()));`
`6292`	`6292`
`6293`	`6293`	`/**************************************************`
`6294`	`6294`	`7785`
Original file line number	Diff line number	Diff line change
`@@ -551,15 +551,15 @@ void test2781()`
`551`	`551`	`}`
`552`	`552`
`553`	`553`	`eval = 0;`
`554`		`- foreach (i, e; tup(tup((eval++, 10), 3.14), tup("str", [1,2])))`
	`554`	`+ foreach (i, e; tup(tup((){eval++; return 10;}(), 3.14), tup("str", [1,2])))`
`555`	`555`	`{`
`556`	`556`	`static if (i == 0) assert(e == tup(10, 3.14));`
`557`	`557`	`static if (i == 1) assert(e == tup("str", [1,2]));`
`558`	`558`	`}`
`559`	`559`	`assert(eval == 1);`
`560`	`560`
`561`	`561`	`eval = 0;`
`562`		`- foreach (i, e; tup((eval++,10), tup(3.14, tup("str", tup([1,2])))))`
	`562`	`+ foreach (i, e; tup((){eval++; return 10;}(), tup(3.14, tup("str", tup([1,2])))))`
`563`	`563`	`{`
`564`	`564`	`static if (i == 0) assert(e == 10);`
`565`	`565`	`static if (i == 1) assert(e == tup(3.14, tup("str", tup([1,2]))));`
`@@ -836,7 +836,7 @@ void test6369c()`
`836`	`836`	`void test6369d()`
`837`	`837`	`{`
`838`	`838`	`int eval = 0;`
`839`		`- Seq!(int, string) t = tup((++eval, 10), "str");`
	`839`	`+ Seq!(int, string) t = tup((){++eval; return 10;}(), "str");`
`840`	`840`	`assert(eval == 1);`
`841`	`841`	`assert(t[0] == 10);`
`842`	`842`	`assert(t[1] == "str");`
Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,8 @@ static assert(bazra(14)==64);`
`136`	`136`
`137`	`137`	`void moreCommaTests()`
`138`	`138`	`{`
`139`		`- auto k = (containsAsm(), containsAsm());`
	`139`	`+ (containsAsm(), containsAsm());`
	`140`	`+ auto k = containsAsm();`
`140`	`141`	`for (int i=0; i< k^^2; i+=StructWithCtor(1).n) {}`
`141`	`142`	`}`
`142`	`143`