• Andrej Mitrovic (27/27) Dec 16 2013 Here's some improper code that is not checked properly by SList
• monarch_dodra (9/40) Dec 17 2013 Didn't I reply to this already?
• Andrej Mitrovic (5/12) Dec 17 2013 Sure: version(assert) SList owner;
• Jonathan M Davis (4/8) Dec 17 2013 Sorry. I keep meaning to look at it, but I'm so busy these days that I h...

Andrej Mitrovic <andrej.mitrovich gmail.com> writes:

Here's some improper code that is not checked properly by SList
(meaning no assertions, not even in debug mode):

-----
import std.stdio;
import std.container;

void main()
{
    auto s1 = SList!string(["a", "b", "d"]);
    auto s2 = SList!string(["a", "b", "d"]);

    // note the s1[] instead of s2[]!
    s2.insertAfter(std.range.take(s1[], 2), "c");

    writeln(s1[]);  // updated s1, not s2!
    writeln(s2[]);  // s2 stays the same.
}
-----

Note the docs of insertAfter:

Inserts $(D stuff) after range $(D r), which must be a range
previously extracted from this container.

Well clearly we have a range extracted from one list being used to add
items to a different list, but SList makes no checks at all to verify
the range belongs to it. I had a look at DCollections[1], and it
stores an "owner" pointer (or reference) in the range type when its
extracted from a linked-list, so it can do these types of sanity
checks.

So shouldn't we add this to SList? We'd have to add an owner field to
its internal "Range" struct.

[1] : https://github.com/schveiguy/dcollections

"monarch_dodra" <monarchdodra gmail.com> writes:

On Monday, 16 December 2013 at 21:57:52 UTC, Andrej Mitrovic 
wrote:
 Here's some improper code that is not checked properly by SList
 (meaning no assertions, not even in debug mode):

 -----
 import std.stdio;
 import std.container;

 void main()
 {
     auto s1 = SList!string(["a", "b", "d"]);
     auto s2 = SList!string(["a", "b", "d"]);

     // note the s1[] instead of s2[]!
     s2.insertAfter(std.range.take(s1[], 2), "c");

     writeln(s1[]);  // updated s1, not s2!
     writeln(s2[]);  // s2 stays the same.
 }
 -----

 Note the docs of insertAfter:

 Inserts $(D stuff) after range $(D r), which must be a range
 previously extracted from this container.

 Well clearly we have a range extracted from one list being used 
 to add
 items to a different list, but SList makes no checks at all to 
 verify
 the range belongs to it. I had a look at DCollections[1], and it
 stores an "owner" pointer (or reference) in the range type when 
 its
 extracted from a linked-list, so it can do these types of sanity
 checks.

 So shouldn't we add this to SList? We'd have to add an owner 
 field to
 its internal "Range" struct.

 [1] : https://github.com/schveiguy/dcollections

Didn't I reply to this already?

Anyways... what I said is that we can add the test in 
non-release, but I see it as assertive code, so it can be removed 
in release.

I'd be more afraid of DList's current semantics: Neither value 
nor ref. I've been trying to fix it for more than a year now:

https://github.com/D-Programming-Language/phobos/pull/953

Andrej Mitrovic <andrej.mitrovich gmail.com> writes:

On 12/17/13, monarch_dodra <monarchdodra gmail.com> wrote:
 Didn't I reply to this already?

dlang.org was down recently, maybe the forum was as well. :)

 Anyways... what I said is that we can add the test in
 non-release, but I see it as assertive code, so it can be removed
 in release.

Sure: version(assert) SList owner;

Works for me.

 I'd be more afraid of DList's current semantics: Neither value
 nor ref. I've been trying to fix it for more than a year now:

 https://github.com/D-Programming-Language/phobos/pull/953

I'll take a look.

"Jonathan M Davis" <jmdavisProg gmx.com> writes:

On Tuesday, December 17, 2013 15:52:35 monarch_dodra wrote:
 I'd be more afraid of DList's current semantics: Neither value
 nor ref. I've been trying to fix it for more than a year now:
 
 https://github.com/D-Programming-Language/phobos/pull/953

Sorry. I keep meaning to look at it, but I'm so busy these days that I haven't 
had time to do much of anything with D. *sigh*

- Jonathan M Davis