Thread overview
Shouldn't SList in std.container hold an owner pointer to verify a range belongs to the list?
Dec 16, 2013
Andrej Mitrovic
Dec 17, 2013
monarch_dodra
Dec 17, 2013
Andrej Mitrovic
Dec 17, 2013
Jonathan M Davis
December 16, 2013
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
December 17, 2013
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
December 17, 2013
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.
December 17, 2013
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