Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue 7444 - Require [] for array copies too #702

Merged
merged 2 commits into from

5 participants

@9rnsr
Collaborator

http://d.puremagic.com/issues/show_bug.cgi?id=7444

Deprecate ambiguity between slice assignment and element-wise one.
See test/runnable/assignable.d

Requires: D-Programming-Language/druntime#314 (merged)
Requires: D-Programming-Language/phobos#960

@9rnsr 9rnsr closed this
@9rnsr 9rnsr reopened this
@andralex
Owner

This looks good, and I like how you handled the covariance a lot. Kudos to @9rnsr! Now I know you'll hate this, but we can't pull this in without adequate documentation. Could you please find the time to write a draft?

@yebblies
Collaborator

Fails to compile druntime

@9rnsr
Collaborator

I've posted a pull request which fixes druntime code based on this enhancement.
See D-Programming-Language/druntime#314 .

@braddr braddr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@WalterBright

As I wrote on the bugzilla entry, I'm not sure this is worth code breakage.

@WalterBright

Maybe do it as a warning?

@9rnsr
Collaborator

OK. I changed deprecation errors to warning.
And I posted an additional Phobos pull #960 (I've completely forgotten to post it).

@9rnsr 9rnsr referenced this pull request in D-Programming-Language/druntime
Merged

Additional fix Issue 7444 in Posix platforms #352

@9rnsr
Collaborator

Comemnt to re-run auto tester.

@WalterBright WalterBright merged commit ba1009c into from
@AndrejMitrovic
Collaborator

Can we improve the error message here? Saying one syntax is better than the other without a reason in the error message has no meaning.

Also the message itself is unpleasant:

void main()
{
    int[] a;
    int[] b;
    a[] = b;
}
test.d(18): Warning: explicit element-wise assignment a[] = (b)[] is better than a[] = b

What's with the parenthesized b?

Another one:

void main()
{
    char[3] dst;
    char[3] src = [1, 2, 3];
    char* ptr = &dst[0];
    ptr[0 .. 3] = src;
}
Warning: explicit element-wise assignment ptr[cast(uint)0..cast(uint)3] = (src)[] is better than ptr[cast(uint)0..cast(uint)3] = src

This is an extremely ugly diagnostic, it needs to improve.

@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 102 additions and 0 deletions.
  1. +45 −0 src/expression.c
  2. +57 −0 test/fail_compilation/warn7444.d
View
45 src/expression.c
@@ -10786,6 +10786,20 @@ Expression *AssignExp::semantic(Scope *sc)
e2 = new SliceExp(e2->loc, e2, NULL, NULL);
e2 = e2->semantic(sc);
}
+ else if (global.params.warnings && !global.gag && op == TOKassign &&
+ e2->op != TOKarrayliteral && e2->op != TOKstring)
+ { // Disallow sa = da (Converted to sa[] = da[])
+ // Disallow sa = e (Converted to sa[] = e)
+ const char* e1str = e1->toChars();
+ const char* e2str = e2->toChars();
+ if (e2->op == TOKslice || t2->implicitConvTo(t1->nextOf()))
+ warning("explicit element-wise assignment (%s)[] = %s is better than %s = %s",
+ e1str, e2str, e1str, e2str);
+ else
+ warning("explicit element-wise assignment (%s)[] = (%s)[] is better than %s = %s",
+ e1str, e2str, e1str, e2str);
+ return new ErrorExp();
+ }
// Convert e1 to e1[]
Expression *e = new SliceExp(e1->loc, e1, NULL, NULL);
@@ -10870,6 +10884,24 @@ Expression *AssignExp::semantic(Scope *sc)
{
checkPostblit(e2->loc, t2->nextOf());
}
+ if (global.params.warnings && !global.gag && op == TOKassign &&
+ e2->op != TOKslice && e2->op != TOKassign &&
+ e2->op != TOKarrayliteral && e2->op != TOKstring &&
+ !(e2->op == TOKadd || e2->op == TOKmin ||
+ e2->op == TOKmul || e2->op == TOKdiv ||
+ e2->op == TOKmod || e2->op == TOKxor ||
+ e2->op == TOKand || e2->op == TOKor ||
+ #if DMDV2
+ e2->op == TOKpow ||
+ #endif
+ e2->op == TOKtilde || e2->op == TOKneg))
+ {
+ const char* e1str = e1->toChars();
+ const char* e2str = e2->toChars();
+ warning("explicit element-wise assignment %s = (%s)[] is better than %s = %s",
+ e1str, e2str, e1str, e2str);
+ return new ErrorExp();
+ }
if (op == TOKconstruct)
e2 = e2->castTo(sc, e1->type->constOf());
else
@@ -10877,6 +10909,19 @@ Expression *AssignExp::semantic(Scope *sc)
}
else
{
+ if (global.params.warnings && !global.gag && op == TOKassign &&
+ t1->ty == Tarray && t2->ty == Tsarray &&
+ e2->op != TOKslice && //e2->op != TOKarrayliteral &&
+ t2->implicitConvTo(t1))
+ { // Disallow ar[] = sa (Converted to ar[] = sa[])
+ // Disallow da = sa (Converted to da = sa[])
+ const char* e1str = e1->toChars();
+ const char* e2str = e2->toChars();
+ warning("explicit %s assignment %s = (%s)[] is better than %s = %s",
+ e1->op == TOKslice ? "element-wise" : "slice",
+ e1str, e2str, e1str, e2str);
+ return new ErrorExp();
+ }
e2 = e2->implicitCastTo(sc, e1->type);
}
if (e2->op == TOKerror)
View
57 test/fail_compilation/warn7444.d
@@ -0,0 +1,57 @@
+// REQUIRED_ARGS: -w
+// PERMUTE_ARGS:
+
+/*
+TEST_OUTPUT:
+---
+fail_compilation/warn7444.d(30): Warning: explicit element-wise assignment (sa)[] = e is better than sa = e
+fail_compilation/warn7444.d(32): Error: cannot implicitly convert expression (e) of type int to int[]
+fail_compilation/warn7444.d(37): Warning: explicit element-wise assignment (sa)[] = sa[] is better than sa = sa[]
+fail_compilation/warn7444.d(38): Warning: explicit element-wise assignment sa[] = (sa)[] is better than sa[] = sa
+fail_compilation/warn7444.d(41): Warning: explicit element-wise assignment (sa)[] = (da)[] is better than sa = da
+fail_compilation/warn7444.d(42): Warning: explicit element-wise assignment (sa)[] = da[] is better than sa = da[]
+fail_compilation/warn7444.d(43): Warning: explicit element-wise assignment sa[] = (da)[] is better than sa[] = da
+fail_compilation/warn7444.d(47): Warning: explicit slice assignment da = (sa)[] is better than da = sa
+fail_compilation/warn7444.d(49): Warning: explicit element-wise assignment da[] = (sa)[] is better than da[] = sa
+fail_compilation/warn7444.d(54): Warning: explicit element-wise assignment da[] = (da)[] is better than da[] = da
+---
+*/
+
+void test7444()
+{
+ int[2] sa;
+ int[] da;
+ int e;
+
+ {
+ // X: Changed accepts-invalid to rejects-invalid by this issue
+ // a: slice assginment
+ // b: element-wise assignment
+ sa = e; // X
+ sa[] = e; // b
+ da = e;
+ da[] = e; // b
+
+ // lhs is static array
+ sa = sa; // b == identity assign
+ sa = sa[]; // X
+ sa[] = sa; // X
+ sa[] = sa[]; // b
+
+ sa = da; // X
+ sa = da[]; // X
+ sa[] = da; // X
+ sa[] = da[]; // b
+
+ // lhs is dynamic array
+ da = sa; // X
+ da = sa[]; // a
+ da[] = sa; // X
+ da[] = sa[]; // b
+
+ da = da; // a == identity assign
+ da = da[]; // a
+ da[] = da; // X
+ da[] = da[]; // b
+ }
+}
Something went wrong with that request. Please try again.