From 8d314b5ffbeb67adf27c68e11976a1d63917717b Mon Sep 17 00:00:00 2001
From: John Colvin <john.loughran.colvin@gmail.com>
Date: Thu, 22 Mar 2018 17:58:42 +0000
Subject: [PATCH] Fix Issue 18525: std.algorithm.mutate.remove should work on
 mutable strings

---
 std/algorithm/mutation.d | 411 +++++++++++++++++++++++++++------------
 1 file changed, 287 insertions(+), 124 deletions(-)

diff --git a/std/algorithm/mutation.d b/std/algorithm/mutation.d
index a31da0f01d3..a171b9d7a16 100644
--- a/std/algorithm/mutation.d
+++ b/std/algorithm/mutation.d
@@ -78,7 +78,8 @@ T2=$(TR $(TDNW $(LREF $1)) $(TD $+))
 module std.algorithm.mutation;
 
 import std.range.primitives;
-import std.traits : isArray, isAssignable, isBlitAssignable, isNarrowString, Unqual, isSomeChar;
+import std.traits : isArray, isAssignable, isBlitAssignable, isNarrowString,
+       Unqual, isSomeChar, isMutable;
 // FIXME
 import std.typecons; // : tuple, Tuple;
 
@@ -1787,13 +1788,143 @@ Returns:
     a range containing all of the elements of range with offset removed
  */
 Range remove
-(SwapStrategy s = SwapStrategy.stable, Range, Offset...)
+(SwapStrategy s = SwapStrategy.stable, Range, Offset ...)
 (Range range, Offset offset)
-if (s != SwapStrategy.stable
-    && isBidirectionalRange!Range
-    && hasLvalueElements!Range
-    && hasLength!Range
-    && Offset.length >= 1)
+if (Offset.length >= 1)
+{
+    static if (isNarrowString!Range)
+    {
+        static assert(isMutable!(typeof(range[0])),
+                "Elements must be mutable to remove");
+        static assert(s == SwapStrategy.stable,
+                "Only stable removing can be done for character arrays");
+        return removeStableString(range, offset);
+    }
+    else
+    {
+        static assert(isBidirectionalRange!Range,
+                "Range must be bidirectional");
+        static assert(hasLvalueElements!Range,
+                "Range must have Lvalue elements (see std.range.hasLvalueElements)");
+
+        static if (s == SwapStrategy.unstable)
+        {
+            static assert(hasLength!Range,
+                    "Range must have `length` for unstable remove");
+            return removeUnstable(range, offset);
+        }
+        else static if (s == SwapStrategy.stable)
+            return removeStable(range, offset);
+        else
+            static assert(false,
+                    "Only SwapStrategy.stable and SwapStrategy.unstable are supported");
+    }
+}
+
+///
+@safe pure unittest
+{
+    import std.typecons : tuple;
+
+    auto a = [ 0, 1, 2, 3, 4, 5 ];
+    assert(remove!(SwapStrategy.stable)(a, 1) == [ 0, 2, 3, 4, 5 ]);
+    a = [ 0, 1, 2, 3, 4, 5 ];
+    assert(remove!(SwapStrategy.stable)(a, 1, 3) == [ 0, 2, 4, 5] );
+    a = [ 0, 1, 2, 3, 4, 5 ];
+    assert(remove!(SwapStrategy.stable)(a, 1, tuple(3, 6)) == [ 0, 2 ]);
+
+    a = [ 0, 1, 2, 3, 4, 5 ];
+    assert(remove!(SwapStrategy.unstable)(a, 1) == [0, 5, 2, 3, 4]);
+    a = [ 0, 1, 2, 3, 4, 5 ];
+    assert(remove!(SwapStrategy.unstable)(a, tuple(1, 4)) == [0, 5, 4]);
+}
+
+@safe unittest
+{
+    import std.exception : assertThrown;
+    import std.range;
+
+    // http://d.puremagic.com/issues/show_bug.cgi?id=10173
+    int[] test = iota(0, 10).array();
+    assertThrown(remove!(SwapStrategy.stable)(test, tuple(2, 4), tuple(1, 3)));
+    assertThrown(remove!(SwapStrategy.unstable)(test, tuple(2, 4), tuple(1, 3)));
+    assertThrown(remove!(SwapStrategy.stable)(test, 2, 4, 1, 3));
+    assertThrown(remove!(SwapStrategy.unstable)(test, 2, 4, 1, 3));
+}
+
+@safe unittest
+{
+    import std.range;
+    int[] a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.stable)(a, 1) ==
+        [ 0, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]);
+
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.unstable)(a, 0, 10) ==
+           [ 9, 1, 2, 3, 4, 5, 6, 7, 8 ]);
+
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.unstable)(a, 0, tuple(9, 11)) ==
+            [ 8, 1, 2, 3, 4, 5, 6, 7 ]);
+    // http://d.puremagic.com/issues/show_bug.cgi?id=5224
+    a = [ 1, 2, 3, 4 ];
+    assert(remove!(SwapStrategy.unstable)(a, 2) ==
+           [ 1, 2, 4 ]);
+
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.stable)(a, 1, 5) ==
+        [ 0, 2, 3, 4, 6, 7, 8, 9, 10 ]);
+
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.stable)(a, 1, 3, 5)
+            == [ 0, 2, 4, 6, 7, 8, 9, 10]);
+    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
+    assert(remove!(SwapStrategy.stable)(a, 1, tuple(3, 5))
+            == [ 0, 2, 5, 6, 7, 8, 9, 10]);
+
+    a = iota(0, 10).array();
+    assert(remove!(SwapStrategy.unstable)(a, tuple(1, 4), tuple(6, 7))
+            == [0, 9, 8, 7, 4, 5]);
+}
+
+@safe unittest
+{
+    // Issue 11576
+    auto arr = [1,2,3];
+    arr = arr.remove!(SwapStrategy.unstable)(2);
+    assert(arr == [1,2]);
+
+}
+
+@safe unittest
+{
+    import std.range;
+    // Bug# 12889
+    int[1][] arr = [[0], [1], [2], [3], [4], [5], [6]];
+    auto orig = arr.dup;
+    foreach (i; iota(arr.length))
+    {
+        assert(orig == arr.remove!(SwapStrategy.unstable)(tuple(i,i)));
+        assert(orig == arr.remove!(SwapStrategy.stable)(tuple(i,i)));
+    }
+}
+
+@safe unittest
+{
+    char[] chars = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'];
+    remove(chars, 4);
+    assert(chars == ['a', 'b', 'c', 'd', 'f', 'g', 'h', 'h']);
+
+    char[] bigChars = "∑œ∆¬é˚˙ƒé∂ß¡¡".dup;
+    assert(remove(bigChars, tuple(4, 6), 8) == ("∑œ∆¬˙ƒ∂ß¡¡"));
+
+    import std.exception : assertThrown;
+    assertThrown(remove(bigChars.dup, 1, 0));
+    assertThrown(remove(bigChars.dup, tuple(4, 3)));
+}
+
+private Range removeUnstable(Range, Offset...)(Range range, Offset offset)
 {
     Tuple!(size_t, "pos", size_t, "len")[offset.length] blackouts;
     foreach (i, v; offset)
@@ -1872,14 +2003,7 @@ if (s != SwapStrategy.stable
     return range;
 }
 
-/// Ditto
-Range remove
-(SwapStrategy s = SwapStrategy.stable, Range, Offset...)
-(Range range, Offset offset)
-if (s == SwapStrategy.stable
-    && isBidirectionalRange!Range
-    && hasLvalueElements!Range
-    && Offset.length >= 1)
+private Range removeStable(Range, Offset...)(Range range, Offset offset)
 {
     auto result = range;
     auto src = range, tgt = range;
@@ -1923,93 +2047,61 @@ if (s == SwapStrategy.stable
     return result;
 }
 
-///
-@safe pure unittest
+private Range removeStableString(Range, Offset...)(Range range, Offset offsets)
 {
-    import std.typecons : tuple;
+    import std.utf : stride;
+    size_t charIdx = 0;
+    size_t dcharIdx = 0;
+    size_t charShift = 0;
 
-    auto a = [ 0, 1, 2, 3, 4, 5 ];
-    assert(remove!(SwapStrategy.stable)(a, 1) == [ 0, 2, 3, 4, 5 ]);
-    a = [ 0, 1, 2, 3, 4, 5 ];
-    assert(remove!(SwapStrategy.stable)(a, 1, 3) == [ 0, 2, 4, 5] );
-    a = [ 0, 1, 2, 3, 4, 5 ];
-    assert(remove!(SwapStrategy.stable)(a, 1, tuple(3, 6)) == [ 0, 2 ]);
-
-    a = [ 0, 1, 2, 3, 4, 5 ];
-    assert(remove!(SwapStrategy.unstable)(a, 1) == [0, 5, 2, 3, 4]);
-    a = [ 0, 1, 2, 3, 4, 5 ];
-    assert(remove!(SwapStrategy.unstable)(a, tuple(1, 4)) == [0, 5, 4]);
-}
-
-@safe unittest
-{
-    import std.exception : assertThrown;
-    import std.range;
-
-    // http://d.puremagic.com/issues/show_bug.cgi?id=10173
-    int[] test = iota(0, 10).array();
-    assertThrown(remove!(SwapStrategy.stable)(test, tuple(2, 4), tuple(1, 3)));
-    assertThrown(remove!(SwapStrategy.unstable)(test, tuple(2, 4), tuple(1, 3)));
-    assertThrown(remove!(SwapStrategy.stable)(test, 2, 4, 1, 3));
-    assertThrown(remove!(SwapStrategy.unstable)(test, 2, 4, 1, 3));
-}
-
-@safe unittest
-{
-    import std.range;
-    int[] a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.stable)(a, 1) ==
-        [ 0, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]);
-
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.unstable)(a, 0, 10) ==
-           [ 9, 1, 2, 3, 4, 5, 6, 7, 8 ]);
+    void skipOne()
+    {
+        charIdx += stride(range[charIdx .. $]);
+        ++dcharIdx;
+    }
 
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.unstable)(a, 0, tuple(9, 11)) ==
-            [ 8, 1, 2, 3, 4, 5, 6, 7 ]);
-    // http://d.puremagic.com/issues/show_bug.cgi?id=5224
-    a = [ 1, 2, 3, 4 ];
-    assert(remove!(SwapStrategy.unstable)(a, 2) ==
-           [ 1, 2, 4 ]);
+    void copyBackOne()
+    {
+        auto encodedLen = stride(range[charIdx .. $]);
+        foreach (j; charIdx .. charIdx + encodedLen)
+            range[j - charShift] = range[j];
+        charIdx += encodedLen;
+        ++dcharIdx;
+    }
 
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.stable)(a, 1, 5) ==
-        [ 0, 2, 3, 4, 6, 7, 8, 9, 10 ]);
+    foreach (pass, i; offsets)
+    {
+        static if (is(typeof(i[0])) && is(typeof(i[1])))
+        {
+            auto from = i[0];
+            auto delta = i[1] - i[0];
+        }
+        else
+        {
+            auto from = i;
+            enum delta = 1;
+        }
 
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.stable)(a, 1, 3, 5)
-            == [ 0, 2, 4, 6, 7, 8, 9, 10]);
-    a = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ];
-    assert(remove!(SwapStrategy.stable)(a, 1, tuple(3, 5))
-            == [ 0, 2, 5, 6, 7, 8, 9, 10]);
+        import std.exception : enforce;
+        enforce(dcharIdx <= from && delta >= 0,
+                "remove(): incorrect ordering of elements to remove");
 
-    a = iota(0, 10).array();
-    assert(remove!(SwapStrategy.unstable)(a, tuple(1, 4), tuple(6, 7))
-            == [0, 9, 8, 7, 4, 5]);
-}
+        while (dcharIdx < from)
+            static if (pass == 0)
+                skipOne();
+            else
+                copyBackOne();
 
-@safe unittest
-{
-    // Issue 11576
-    auto arr = [1,2,3];
-    arr = arr.remove!(SwapStrategy.unstable)(2);
-    assert(arr == [1,2]);
+        auto mark = charIdx;
+        while (dcharIdx < from + delta)
+            skipOne();
+        charShift += charIdx - mark;
+    }
 
-}
+    foreach (i; charIdx .. range.length)
+        range[i - charShift] = range[i];
 
-@safe unittest
-{
-    import std.range;
-    // Bug# 12889
-    int[1][] arr = [[0], [1], [2], [3], [4], [5], [6]];
-    auto orig = arr.dup;
-    foreach (i; iota(arr.length))
-    {
-        assert(orig == arr.remove!(SwapStrategy.unstable)(tuple(i,i)));
-        assert(orig == arr.remove!(SwapStrategy.stable)(tuple(i,i)));
-    }
+    return range[0 .. $ - charShift];
 }
 
 /**
@@ -2023,49 +2115,38 @@ order is preserved. Returns the filtered range.
 
 Params:
     range = a bidirectional ranges with lvalue elements
+        or mutable character arrays
 
 Returns:
     the range with all of the elements where `pred` is `true`
     removed
 */
-Range remove(alias pred, SwapStrategy s = SwapStrategy.stable, Range)
-(Range range)
-if (isBidirectionalRange!Range
-    && hasLvalueElements!Range)
+Range remove(alias pred, SwapStrategy s = SwapStrategy.stable, Range)(Range range)
 {
     import std.functional : unaryFun;
-    auto result = range;
-    static if (s != SwapStrategy.stable)
+    alias pred_ = unaryFun!pred;
+    static if (isNarrowString!Range)
     {
-        for (;!range.empty;)
-        {
-            if (!unaryFun!pred(range.front))
-            {
-                range.popFront();
-                continue;
-            }
-            move(range.back, range.front);
-            range.popBack();
-            result.popBack();
-        }
+        static assert(isMutable!(typeof(range[0])),
+                "Elements must be mutable to remove");
+        static assert(s == SwapStrategy.stable,
+                "Only stable removing can be done for character arrays");
+        return removePredString!pred_(range);
     }
     else
     {
-        auto tgt = range;
-        for (; !range.empty; range.popFront())
-        {
-            if (unaryFun!(pred)(range.front))
-            {
-                // yank this guy
-                result.popBack();
-                continue;
-            }
-            // keep this guy
-            move(range.front, tgt.front);
-            tgt.popFront();
-        }
+        static assert(isBidirectionalRange!Range,
+                "Range must be bidirectional");
+        static assert(hasLvalueElements!Range,
+                "Range must have Lvalue elements (see std.range.hasLvalueElements)");
+        static if (s == SwapStrategy.unstable)
+            return removePredUnstable!pred_(range);
+        else static if (s == SwapStrategy.stable)
+            return removePredStable!pred_(range);
+        else
+            static assert(false,
+                    "Only SwapStrategy.stable and SwapStrategy.unstable are supported");
     }
-    return result;
 }
 
 ///
@@ -2172,6 +2253,88 @@ if (isBidirectionalRange!Range
     }
 }
 
+@safe unittest
+{
+    char[] chars = "abcdefg".dup;
+    assert(chars.remove!(dc => dc == 'c' || dc == 'f') == "abdeg");
+    assert(chars == "abdegfg");
+
+    assert(chars.remove!"a == 'd'" == "abegfg");
+
+    char[] bigChars = "¥^¨^©é√∆π".dup;
+    assert(bigChars.remove!(dc => dc == "¨"d[0] || dc == "é"d[0]) ==  "¥^^©√∆π");
+}
+
+private Range removePredUnstable(alias pred, Range)(Range range)
+{
+    auto result = range;
+    for (;!range.empty;)
+    {
+        if (!pred(range.front))
+        {
+            range.popFront();
+            continue;
+        }
+        move(range.back, range.front);
+        range.popBack();
+        result.popBack();
+    }
+    return result;
+}
+
+private Range removePredStable(alias pred, Range)(Range range)
+{
+    auto result = range;
+    auto tgt = range;
+    for (; !range.empty; range.popFront())
+    {
+        if (pred(range.front))
+        {
+            // yank this guy
+            result.popBack();
+            continue;
+        }
+        // keep this guy
+        move(range.front, tgt.front);
+        tgt.popFront();
+    }
+    return result;
+}
+
+private Range removePredString(alias pred, SwapStrategy s = SwapStrategy.stable, Range)
+(Range range)
+{
+    import std.utf : decode;
+    import std.functional : unaryFun;
+
+    alias pred_ = unaryFun!pred;
+
+    size_t charIdx = 0;
+    size_t charShift = 0;
+    while (charIdx < range.length)
+    {
+        size_t start = charIdx;
+        if (pred_(decode(range, charIdx)))
+        {
+            charShift += charIdx - start;
+            break;
+        }
+    }
+    while (charIdx < range.length)
+    {
+        size_t start = charIdx;
+        auto doRemove = pred_(decode(range, charIdx));
+        auto encodedLen = charIdx - start;
+        if (doRemove)
+            charShift += encodedLen;
+        else
+            foreach (i; start .. charIdx)
+                range[i - charShift] = range[i];
+    }
+
+    return range[0 .. $ - charShift];
+}
+
 // reverse
 /**
 Reverses `r` in-place.  Performs `r.length / 2` evaluations of `swap`.