www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 1339] New: Invariant/const-ness is broken by built-in array properties

reply d-bugmail puremagic.com writes:
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...


-- 
Jul 12 2007
next sibling parent d-bugmail puremagic.com writes:
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.


-- 
Jan 10 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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.


-- 
Feb 18 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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.


-- 
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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. --
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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. --
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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?

is 2.023. --
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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.


-- 
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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?) --
Feb 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Mar 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                 CC|                            |clugdbug yahoo.com.au


--- Comment #10 from Don <clugdbug yahoo.com.au> 2010-05-06 13:33:54 PDT ---
Dunno why I bothered with this one, since we should just ditch .sort and
.reverse.
But anyway...

PATCH: expression.c  DotIdExp::semantic(), around line 6113.

    else if (t1b->ty == Tarray ||
             t1b->ty == Tsarray ||
             t1b->ty == Taarray)
    {   /* If ident is not a valid property, rewrite:
         *   e1.ident
         * as:
         *   .ident(e1)
         */
        TypeArray *tarr = (TypeArray *)t1b;
        if (ident == Id::sort && !tarr->next->isMutable()) {
            error(".sort only applies to mutable arrays");
            return new ErrorExp();
        }
        if (ident == Id::reverse && !tarr->next->isMutable()) {
            error(".reverse only applies to mutable arrays");
            return new ErrorExp();
        }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 06 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339



--- Comment #11 from Steven Schveighoffer <schveiguy yahoo.com> 2010-05-06
13:38:49 PDT ---
Don, while you are looking at this, have a look at related bug 3550

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 06 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc


--- Comment #12 from bearophile_hugs eml.cc 2010-05-06 13:59:43 PDT ---
I think array reverse can be kept.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 06 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339



--- Comment #13 from Don <clugdbug yahoo.com.au> 2010-05-10 01:44:42 PDT ---
There's a bit of code in druntime, in rt/adi.d which is affected by this.

extern (C) long _adSortChar(char[] a)
{
    if (a.length > 1)
    {
-        dstring da = toUTF32(a);
+        dchar [] da = cast(dchar[])(toUTF32(a));
        da.sort;


and likewise for _adSortWChar.

But it'd probably be better to modify rt/util/utf/toUTF32().

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 10 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339



--- Comment #14 from bearophile_hugs eml.cc 2011-04-06 16:56:23 PDT ---
The same happens with enum arrays:

enum int[] data = [3, 1, 2];
void main() {
    data.sort;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 06 2011
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1339



--- Comment #15 from Stewart Gordon <smjg iname.com> 2011-04-09 08:13:36 PDT ---
(In reply to comment #14)
 The same happens with enum arrays:
 
 enum int[] data = [3, 1, 2];
 void main() {
     data.sort;
 }

Array enums have enough problems of their own. See issue 2331, issue 2414 and probably others. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Apr 09 2011