diff --git a/changelog/comma-deprecation-error.dd b/changelog/comma-deprecation-error.dd new file mode 100644 index 000000000000..f892b7268af7 --- /dev/null +++ b/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 + --- +) diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index 26ba535f4995..c281fb8527c3 100644 --- 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("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); diff --git a/test/compilable/interpret3.d b/test/compilable/interpret3.d index 386743e6ddc4..52abecb28560 100644 --- 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 index 3296046e234b..8570faf1407d 100644 --- 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: 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); +} diff --git a/test/fail_compilation/diag10862.d b/test/fail_compilation/diag10862.d index e03307b55382..6220697fe6d1 100644 --- 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: 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` diff --git a/test/fail_compilation/fail13902.d b/test/fail_compilation/fail13902.d index fbe4b4a6713f..77ab22eda1d1 100644 --- 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: 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( diff --git a/test/fail_compilation/ice10949.d b/test/fail_compilation/ice10949.d index e81cf9795493..45b18e0de5ab 100644 --- 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: 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)` diff --git a/test/runnable/aliasthis.d b/test/runnable/aliasthis.d index 8cd005176921..eeb6860293eb 100644 --- a/test/runnable/aliasthis.d +++ b/test/runnable/aliasthis.d @@ -551,7 +551,7 @@ 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])); @@ -559,7 +559,7 @@ void test2781() 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 index 10bc47e7e2d8..597bd5eac74d 100644 --- 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 index c86535cbb51d..bb516a26f67a 100644 --- 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 index 2fea5466dd88..8a87a3e92e37 100644 --- 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 index 883d58ac07c2..026a5abfa7b1 100644 --- 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 index 37f7d1bacd2a..0ab5a2a13292 100644 --- 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 index c8ad4649f2cf..8cd9c6ad4824 100644 --- 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 index e2d26e706b64..0525df3ebff0 100644 --- 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 index 21a587dbd4bf..438e87c973a1 100644 --- 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 index a8d98c49e9f2..2299afe80a71 100644 --- 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 index 1735c5af2a70..7b1380668508 100644 --- 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 index e37e2d7a656f..48a1e6553c14 100644 --- 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 index e68e2fc43228..01ced7a758f9 100644 --- a/test/runnable/xtest46.d +++ b/test/runnable/xtest46.d @@ -4684,7 +4684,8 @@ void test10626() double[2] a = v[] * ++z; double[2] b = v[] * --z; double[2] c = v[] * y.u; - double[2] d = v[] * (x[] = 3, x[0]); + x[] = 3; + double[2] d = v[] * x[0]; double[2] e = v[] * (v[] ~ z)[0]; } @@ -7866,7 +7867,7 @@ bool test16022() { enum Type { Colon, Comma } Type type; - return type == Type.Colon, type == Type.Comma; + return type == Type.Comma; } bool test16022_structs() @@ -7879,7 +7880,7 @@ bool test16022_structs() enum Type { Colon = A(0, "zero"), Comma = A(1, "one") } Type type; - return type == Type.Colon, type == Type.Comma; + return type == Type.Comma; } /***************************************************/