https://issues.dlang.org/show_bug.cgi?id=13279

--- Comment #1 from NCrashed@gmail.com ---
Compiler output:
```
core.exception.AssertError@/usr/include/dmd/phobos/std/container/dlist.d(125):
DList.Range: Invalidated state
----------------
/home/ncrashed/dev/d/dmd-test/dmd-test(pure nothrow @nogc @safe void
std.container.dlist.DList!(int).DList.Range.popFront()+0xf3) [0x44298b]
/home/ncrashed/dev/d/dmd-test/dmd-test(_Dmain+0x73) [0x43f683]
/home/ncrashed/dev/d/dmd-test/dmd-test(_D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ6runAllMFZ9__lambda1MFZv+0x1f)
[0x44e097]
/home/ncrashed/dev/d/dmd-test/dmd-test(void rt.dmain2._d_run_main(int, char**,
extern (C) int function(char[][])*).tryExec(scope void delegate())+0x2a)
[0x44dfea]
/home/ncrashed/dev/d/dmd-test/dmd-test(void rt.dmain2._d_run_main(int, char**,
extern (C) int function(char[][])*).runAll()+0x30) [0x44e050]
/home/ncrashed/dev/d/dmd-test/dmd-test(void rt.dmain2._d_run_main(int, char**,
extern (C) int function(char[][])*).tryExec(scope void delegate())+0x2a)
[0x44dfea]
/home/ncrashed/dev/d/dmd-test/dmd-test(_d_run_main+0x193) [0x44df6b]
/home/ncrashed/dev/d/dmd-test/dmd-test(main+0x25) [0x44a2b5]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x39e9021d65]
```

--

https://issues.dlang.org/show_bug.cgi?id=13279

NCrashed@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |NCrashed@gmail.com

--

https://issues.dlang.org/show_bug.cgi?id=13279

--- Comment #2 from Kenji Hara <k.hara.pg@gmail.com> ---
The assert was introduced in: https://github.com/D-Programming-Language/phobos/pull/1954

However, as far as I see, the code modifies dlist during its iteration. By the modification, original range would be invalid and the assertion detects "invalid state".

--

https://issues.dlang.org/show_bug.cgi?id=13279

monarchdodra@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |monarchdodra@gmail.com

--- Comment #3 from monarchdodra@gmail.com ---
(In reply to Kenji Hara from comment #2)
> The assert was introduced in: https://github.com/D-Programming-Language/phobos/pull/1954
> 
> However, as far as I see, the code modifies dlist during its iteration. By the modification, original range would be invalid and the assertion detects "invalid state".

Aye. This triggered a new check that verifies the integrity of the range more thoroughly than before, rather than just letting dangerous state slide.

The issue is not just that the code modifies the range it is iterating on, but more specifically, removing the very node it is currently on.

When the node gets removed, it is now also unlinked, so when the foreach calls "pop" on the removed node, it finds nothing.

While we *could* have this keep working by simply letting removed nodes still
reference their neighbors:
1. We have shown this produces strange iteration behavior sometimes.
2. It makes the assumption that nodes are GC-allocated, and that the unlinked
node isn't destroyed.

But yeah, overall, doing a "foreach" over a container while removing certain of its elements is a *terrible* idea. C++ books have entire tutorial chapters on how to do this, and it applies here perfectly well to D.

Not to mention, that code has linear complexity. In any case, something like this should be more correct (not tested):

//----
DList.Range removeOne(T)(ref DList!T list, T elem)
{
   auto toRemove = list[].find(elem).take(1);
   return list.linearRemove(toRemove);
}

void main()
{
    DList!int list;
    list.insert(1);
    list.insert(2);

    for (auto r = list[] ; !r.empty ; )
    {
        if(r.front == 1)
            r = removeOne(list, elem);
        else
            r.popFront();
    }
}
//----

Or better yet:
//----
DList!T.Range removeOne(T)(ref DList!T list, DList!T.R toRemove )
{
   return list.linearRemove(toRemove.take(1));
}

void main()
{
    DList!int list;
    list.insert(1);
    list.insert(2);

    for (auto r = list[] ; !r.empty ; )
    {
        if(r.front == 1)
            r = removeOne(list, r);
        else
            r.popFront();
    }
}
//----

OK to close this as Invalid?

--

https://issues.dlang.org/show_bug.cgi?id=13279

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #4 from Kenji Hara <k.hara.pg@gmail.com> ---
Thank you for good replying, @monarchdodra.

> OK to close this as Invalid?

I'll do it.

--

https://issues.dlang.org/show_bug.cgi?id=13279

--- Comment #5 from NCrashed@gmail.com ---
Thank you a lot, @monarchdodra! I was part of legacy code and the main idea was "don't touch while it works!". Modifying collection while iterating over is really horrible idea.

Thus my list does much more deletion operations than addition ones, I simply fills new list with remaining elements and swaps two lists at the end of iteration.

Invalid.

--