View mode: basic / threaded / horizontal-split · Log in · Help
February 05, 2012
[Issue 7444] New: Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444

          Summary: Require [] for array copies too
          Product: D
          Version: D2
         Platform: All
       OS/Version: All
           Status: NEW
         Severity: enhancement
         Priority: P2
        Component: DMD
       AssignedTo: nobody@puremagic.com
       ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2012-02-05 11:10:57 PST ---
This is derived from issue 3971 see there for more (messy) discussions.

In 2.058 this compiles:


int[100] foo() {
   int[100] a;
   return a;
}
void main() {
   int[10_000] a, b;
   auto c = new int[10_000];
   a = 1;
   a = b;
   a = c;
   auto d = foo();
}


But for consistency of the vector ops syntax, and to help the programmer spot
where the code is doing a potentially long copy, I suggest to turn that first
program into a compile-time error, and require [] on lvalues everywhere you
perform a vector operation, like when you copy a int[N] on another int[N], etc:


int[100] foo() {
   int[100] a;
   return a;
}
void main() {
   int[10_000] a, b;
   auto c = new int[10_000];
   a[] = 1;
   a[] = b;
   a[] = c;
   auto d[] = foo(); // currently an error
}

- - - - - - - - - - - - - - - -

But note that normally all array ops require a [] even on the rvalue:


void main() {
   int[10_000] a, b;
   a[] += b[];
}



So an alternative stricter proposal is to require [] on the right too:


int[100] foo() {
   int[100] a;
   return a;
}
void main() {
   int[10_000] a, b;
   auto c = new int[10_000];
   auto d = new int[10_000];
   a[] = 1;
   a[] = b[];
   a[] = c[];
   c[] = d[];
   auto e[] = foo()[]; // currently an error
}



It helps the programmer tell apart two different cases, that currently are
usable with the same syntax (from an example by Don):


void main() {
   int[][3] x;
   int[]    y;
   int[]    z;

   x[] = z; // copies just the z pointer
   y[] = z; // copies the elements in z
}



Requiring [] on the right (rvalue) for the copy of many items avoids the
ambiguity:

void main() {
   int[][3] x;
   int[]    y;
   int[]    z;
   x[] = z;
   y[] = z[];
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 05, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444


timon.gehr@gmx.ch changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
                CC|                            |timon.gehr@gmx.ch


--- Comment #1 from timon.gehr@gmx.ch 2012-02-05 15:34:04 PST ---
While I agree that the syntax should be enforced more strictly in general, I
still completely disagree with requiring [] on static array copies. Static
arrays are value types of fixed size, and should be treated as such. Requiring
[] is just wrong.

void foo(int[4] x){}
void foo(int[] y){}

void main(){
   int[4] x, y;
   struct S{int[4] x;}
   S a, b;
   x = y;
   a = b; // why should this work if the above does not?
   foo(x);   // copies, you want this to be an error
   foo(x[]); // calls the other overload, does not copy
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 09, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #2 from Kenji Hara <k.hara.pg@gmail.com> 2012-02-09 05:33:52 PST ---
(In reply to comment #1)
> While I agree that the syntax should be enforced more strictly in general, I
> still completely disagree with requiring [] on static array copies. Static
> arrays are value types of fixed size, and should be treated as such. Requiring
> [] is just wrong.
> 
> void foo(int[4] x){}
> void foo(int[] y){}
> 
> void main(){
>     int[4] x, y;
>     struct S{int[4] x;}
>     S a, b;
>     x = y;
>     a = b; // why should this work if the above does not?
>     foo(x);   // copies, you want this to be an error
>     foo(x[]); // calls the other overload, does not copy
> }

I completely agree with Timon.
When x and y are declared with same type T, x = y is identity assignment, and
it is valid syntax even if T is static array type.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 09, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #3 from Kenji Hara <k.hara.pg@gmail.com> 2012-02-09 06:31:34 PST ---
I'd like to provide an exhaustive test that should compile after fixing the
enhancement suggested by me and timon.

----
// X: Changed accepts-invalid to rejects-invalid by this issue
// a: slice assginment
// b: element-wise assignment

static assert(!__traits(compiles, { sa   = e; }));      // X
static assert( __traits(compiles, { sa[] = e; }));      // b
static assert(!__traits(compiles, { da   = e; }));
static assert( __traits(compiles, { da[] = e; }));      // b

// lhs is static array
static assert( __traits(compiles, { sa   = sa;   }));   // b == identity assign
static assert(!__traits(compiles, { sa   = sa[]; }));   // X
static assert(!__traits(compiles, { sa[] = sa;   }));   // X
static assert( __traits(compiles, { sa[] = sa[]; }));   // b

static assert(!__traits(compiles, { sa   = da;   }));   // X
static assert(!__traits(compiles, { sa   = da[]; }));   // X
static assert( __traits(compiles, { sa[] = da;   }));   // b
static assert( __traits(compiles, { sa[] = da[]; }));   // b

// lhs is dynamic array
static assert(!__traits(compiles, { da   = sa;   }));   // X
static assert( __traits(compiles, { da   = sa[]; }));   // a
static assert(!__traits(compiles, { da[] = sa;   }));   // X
static assert( __traits(compiles, { da[] = sa[]; }));   // b

static assert( __traits(compiles, { da   = da;   }));   // a == identity assign
static assert( __traits(compiles, { da   = da[]; }));   // a
static assert( __traits(compiles, { da[] = da;   }));   // b
static assert( __traits(compiles, { da[] = da[]; }));   // b
----

bearophile says that sa = sa should be rejected and must replace it to sa[] =
sa[].
But I and timon disagree it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 09, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #4 from timon.gehr@gmx.ch 2012-02-09 07:01:17 PST ---
Maybe sa[] = da and da[] = da should be '// X' too.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 10, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444


Kenji Hara <k.hara.pg@gmail.com> changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
          Keywords|                            |pull


--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> 2012-02-10 04:23:32 PST ---
(In reply to comment #4)
> Maybe sa[] = da and da[] = da should be '// X' too.

Hmm, OK. If it is an element-wise assignment, we should add explicit slicing
both side of AssignExp.

I've posted a pull to implement this enhancement:
https://github.com/D-Programming-Language/dmd/pull/702

Updated test:
https://github.com/D-Programming-Language/dmd/pull/702/files#L6R250

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 18, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444


Walter Bright <bugzilla@digitalmars.com> changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
                CC|                            |bugzilla@digitalmars.com


--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> 2012-02-17 23:27:14 PST ---
The enhancement is to disallow:

 sa = da; // use sa[] = da[] instead
 sa = e;  // use sa[] = e instead

I'm not sure this is worth the existing code breakage.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 18, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #7 from bearophile_hugs@eml.cc 2012-02-18 05:02:07 PST ---
(In reply to comment #6)

> I'm not sure this is worth the existing code breakage.

I thin it's worth it, if you want with warning / deprecation / error stages.

But if want, I'll ask for a little voting in the main D newsgroup.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
February 23, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #8 from bearophile_hugs@eml.cc 2012-02-22 16:07:54 PST ---
See why a consistent syntax matters: Issue 7564

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 15, 2012
[Issue 7444] Require [] for array copies too
http://d.puremagic.com/issues/show_bug.cgi?id=7444



--- Comment #9 from github-bugzilla@puremagic.com 2012-10-14 19:19:00 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/a77c82772e7e7b2d1d863b1fb56b614b9d4bc6a1
fix Issue 7444 - Require [] for array copies too

https://github.com/D-Programming-Language/druntime/commit/be3a7fa1bc726b453203c058ff2fa8c81dcfcab1
Merge pull request #314 from 9rnsr/fix7444

Supplemental changes for Issue 7444 - Require [] for array copies too

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
« First   ‹ Prev
1 2 3
Top | Discussion index | About this forum | D home