Hi,

(earlier posting on D.learn, but better test included here)...


dmd.2.067, dmd.2068 (64bit linux)


import std.container: SList;

void main()
{
    SList!int tmp=( 4 );
    SList!int tmp1;

    // Ensure tmp is empty
    tmp.removeAny();
    assert( tmp.empty == true );

    // Both tmp and tmp1 are empty
    // (however tmp1 is not internally initialised)
    tmp.insertAfter( tmp[], 3 );
//    tmp1.insertAfter( tmp1[], 3 );	<====== asserts here.

}

core.exception.AssertError@std/container/slist.d(57): Assertion failure

Changes in phobos mean that you cannot append to an _uninitialised_ empty singly linked list (you can, however, append to an _initialised_ empty linked list.....

bug ?

--ted

On Friday, 14 August 2015 at 06:44:53 UTC, ted wrote:
> Hi,
>
> (earlier posting on D.learn, but better test included here)...
>
>
> dmd.2.067, dmd.2068 (64bit linux)
>
>
> import std.container: SList;
>
> void main()
> {
>     SList!int tmp=( 4 );
>     SList!int tmp1;
>
>     // Ensure tmp is empty
>     tmp.removeAny();
>     assert( tmp.empty == true );
>
>     // Both tmp and tmp1 are empty
>     // (however tmp1 is not internally initialised)
>     tmp.insertAfter( tmp[], 3 );
> //    tmp1.insertAfter( tmp1[], 3 );	<====== asserts here.
>
> }
>
> core.exception.AssertError@std/container/slist.d(57): Assertion failure
>
> Changes in phobos mean that you cannot append to an _uninitialised_ empty singly linked list (you can, however, append to an _initialised_ empty linked list.....
>
> bug ?
>
> --ted

I can confirm that this is a bug but I'm not sure what the "correct" way is to fix it. SList creates a dummy node for the root of the list, but because structs don't allow default constructors, this dummy node is never allocated in the first case. Either we can add lots of null checks to initialize the list, or we can add this line:

@disable this();

Thoughts?

On Friday, 14 August 2015 at 16:28:39 UTC, Xinok wrote:
> I can confirm that this is a bug but I'm not sure what the "correct" way is to fix it. SList creates a dummy node for the root of the list, but because structs don't allow default constructors, this dummy node is never allocated in the first case. Either we can add lots of null checks to initialize the list, or we can add this line:
>
> @disable this();
>
> Thoughts?

Other insert*  functions call the private function SList.initialize() which does the null-check for _root. I am working on a PR adding the missing call in insertAfter - that's a 1 line change. I am not a phobos dev.

On Friday, 14 August 2015 at 18:12:42 UTC, anonymous wrote:
> Other insert*  functions call the private function SList.initialize() which does the null-check for _root. I am working on a PR adding the missing call in insertAfter - that's a 1 line change. I am not a phobos dev.

I would do the null check in the insert* functions themselves. Otherwise, you have the overhead of a function call, assuming the function isn't inlined.

On Friday, 14 August 2015 at 19:44:17 UTC, Xinok wrote:
> On Friday, 14 August 2015 at 18:12:42 UTC, anonymous wrote:
>> Other insert*  functions call the private function SList.initialize() which does the null-check for _root. I am working on a PR adding the missing call in insertAfter - that's a 1 line change. I am not a phobos dev.
>
> I would do the null check in the insert* functions themselves. Otherwise, you have the overhead of a function call, assuming the function isn't inlined.

https://github.com/D-Programming-Language/phobos/pull/3557 may be a better place to discuss this.
Note: I have not the knowledge to look at disassembly and to figure out whether dmd inlined a function call or not.