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

           Summary: Invariant/const-ness is broken by built-in array
                    properties
           Product: D
           Version: 2.002
          Platform: PC
        OS/Version: All
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: kinaba@is.s.u-tokyo.ac.jp


Here's an example:

import std.stdio;
void main() {
        invariant(int)[] x = [1,4,2,3];

        x.sort;
        writefln(x); // [1,2,3,4]

        x.reverse;
        writefln(x); // [4,3,2,1]

        x.length = x.length - 1;
        x.length = x.length + 1;
        writefln(x); // [4,3,2,0]
}

The properties .sort and .reverse should be simply disallowed for invariant arrays. I'm not sure about the .length property, but I think some consideration must be given...


-- 

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


smjg@iname.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |smjg@iname.com




------- Comment #1 from smjg@iname.com  2009-01-10 11:43 -------
This is closely related to bug 2544.


-- 

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





------- Comment #2 from smjg@iname.com  2009-02-18 15:51 -------
Did I mean 2544?  In any case, bug 2093 is one that the .length bit especially certainly is related to.


-- 

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





------- Comment #3 from maxmo@pochta.ru  2009-02-19 02:55 -------
It reallocated array on increasing length. Seems valid.


-- 

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





------- Comment #4 from 2korden@gmail.com  2009-02-19 03:27 -------
(In reply to comment #3)
> It reallocated array on increasing length. Seems valid.
> 

Yes, the length part is fine. Here is slightly modified test case:

import std.stdio;
import std.algorithm;

void main()
{
        immutable(int)[] x = [1, 5, 3, 4];
        auto y = x;

        writefln(y);
        x.sort;
        writefln(y); // immutable variable is modified!
        x.reverse;
        writefln(y);
}

Problem is that sort and reverse work in-place, thus modifying immutable variable. It should either not compile /or/ allocate a copy. I don't like the hidden allocations, so this should be just disallowed, imo.


-- 

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





------- Comment #5 from smjg@iname.com  2009-02-19 03:41 -------
(In reply to comment #4)
> (In reply to comment #3)
>> It reallocated array on increasing length. Seems valid.

What did - the testcase here with checking added, the testcase at bug 2093 or your own?  With which DMD version?

> Problem is that sort and reverse work in-place, thus modifying immutable variable. It should either not compile /or/ allocate a copy. I don't like the hidden allocations, so this should be just disallowed, imo.

Indeed, to either modify in-place or reallocate depending on constancy status would be a confusing inconsistency.  They should be distinct operations. Indeed, you can already do a reallocating sort or reverse by doing

    int[] sorted = x.dup.sort;
    invariant(int)[] = assumeUnique(x.dup.reverse);

so it would be a question of whether it's worth creating syntactic sugar for this.


-- 

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





------- Comment #6 from maxmo@pochta.ru  2009-02-19 04:04 -------
> What did?  With which DMD version?
The first testcase did. DMD version is specified in the bugreport. My version is 2.023.


-- 

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





------- Comment #7 from smjg@iname.com  2009-02-19 13:59 -------
Oh yes.  Increasing .length always reallocates under 2.025, whether const, invariant or not.  The problem is that concatenation doesn't.


-- 

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





------- Comment #8 from 2korden@gmail.com  2009-02-19 18:06 -------
(In reply to comment #7)
> Oh yes.  Increasing .length always reallocates under 2.025, whether const, invariant or not.  The problem is that concatenation doesn't.
> 

Do you say that the following:
char[] s;
for (int i = 0; i < 10_000_000; ++i) {
    s ~= '0';
    // or:
    // s.length = s.length + 1;
    // s[$-1] = '0';
}

will reallocate on *each* iteration?
While this solves one issue, it opens another one - big performance drop
(create new report?)


-- 

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


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com
            Summary|Invariant/const-ness is     |Invariant/const-ness is
                   |broken by built-in array    |broken by built-in array
                   |properties                  |properties sort and reverse


--- Comment #9 from Steven Schveighoffer <schveiguy@yahoo.com> 2010-03-16 20:06:45 PDT ---
The length property problems of this bug are fixed via the patch in bug 3637. This was incorporated in dmd 2.041.

The original test case for the length was invalid.

the code still prints 4 3 2 0, because it is OK to append to an
invariant(int)[].

A valid test that would have failed prior to 2.041:

import std.stdio;
void main()
{
invariant(int)[] x = [1,2,3,4];
auto y = x;
x.length = x.length - 1;
x.length = x.length + 1;
writeln(y); // prints 1 2 3 4 (prior to 2.041 would print 1 2 3 0)
writeln(x); // prints 1 2 3 0
}

The sort and reverse property bugs remain valid bugs.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------