Merge pull request #6115 from wilzbach/13121

Issue 13121 - std.algorithm.joiner should return a bidirectional range if possible

changelog/std-algorithm-iteration-joiner.dd

@@ -0,0 +1,25 @@
+`std.algorithm.iteration.joiner` can now be used for bidirectional ranges
+
+$(REF joiner, std,algorithm,iteration) can now be requested to be a bidirectional range.
+
+---
+import std.algorithm.iteration : joiner;
+import std.range : retro;
+[[1, 2], [3, 4]].joiner.retro; // [4, 3, 2, 1]
+---
+
+A more complex example of inserting a format delimiter fully lazily:
+
+---
+import std.algorithm.comparison : equal;
+import std.range : chain, cycle, iota, only, retro, take, zip;
+import std.format : format;
+
+static immutable number = "12345678";
+static immutable delimiter = ",";
+auto formatted = number.retro
+    .zip(3.iota.cycle.take(number.length))
+    .map!(z => chain(z[0].only, z[1] == 2 ? delimiter : null))
+    .joiner
+    .retro; // "12,345,678"
+---

std/algorithm/iteration.d

@@ -74,6 +74,7 @@ module std.algorithm.iteration;
 import std.functional; // : unaryFun, binaryFun;
 import std.range.primitives;
 import std.traits;
+import std.typecons : Flag;
 
 private template aggregate(fun...)
 if (fun.length >= 1)
@@ -2146,7 +2147,9 @@ Params:
 Returns:
 A range of elements in the joined range. This will be a forward range if
 both outer and inner ranges of `RoR` are forward ranges; otherwise it will
-be only an input range.
+be only an input range. The
+$(REF_ALTTEXT range bidirectionality, isBidirectionalRange, std,range,primitives)
+is propagated if no separator is specified.
 
 See_also:
 $(REF chain, std,range), which chains a sequence of ranges with compatible elements
@@ -2479,50 +2482,33 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
     private:
         RoR _items;
         ElementType!RoR _current;
-        enum prepare =
+        enum isBidirectional = isForwardRange!RoR && isForwardRange!(ElementType!RoR) &&
-        q{
+                               isBidirectionalRange!RoR && isBidirectionalRange!(ElementType!RoR);
-            // Skip over empty subranges.
+        static if (isBidirectional)
-            if (_items.empty) return;
+        {
-            while (_items.front.empty)
+            ElementType!RoR _currentBack;
-            {
+            bool reachedFinalElement;
-                _items.popFront();
+        }
-                if (_items.empty) return;
+
-            }
-            // We cannot export .save method unless we ensure subranges are not
-            // consumed when a .save'd copy of ourselves is iterated over. So
-            // we need to .save each subrange we traverse.
-            static if (isForwardRange!RoR && isForwardRange!(ElementType!RoR))
-                _current = _items.front.save;
-            else
-                _current = _items.front;
-        };
         this(RoR items, ElementType!RoR current)
         {
             _items = items;
             _current = current;
+            static if (isBidirectional && hasNested!Result)
+                _currentBack = typeof(_currentBack).init;
         }
+
     public:
         this(RoR r)
         {
             _items = r;
-            //mixin(prepare); // _current should be initialized in place
 
-            // Skip over empty subranges.
+            static if (isBidirectional && hasNested!Result)
-            while (!_items.empty && _items.front.empty)
+                _currentBack = typeof(_currentBack).init;
-                _items.popFront();
+            // field _current must be initialized in constructor, because it is nested struct
-
+            mixin(popFrontEmptyElements);
-            if (_items.empty)
+            static if (isBidirectional)
-                _current = _current.init;   // set invalid state
+                mixin(popBackEmptyElements);
-            else
-            {
-                // We cannot export .save method unless we ensure subranges are not
-                // consumed when a .save'd copy of ourselves is iterated over. So
-                // we need to .save each subrange we traverse.
-                static if (isForwardRange!RoR && isForwardRange!(ElementType!RoR))
-                    _current = _items.front.save;
-                else
-                    _current = _items.front;
-            }
         }
         static if (isInfinite!RoR)
         {
@@ -2546,16 +2532,39 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
             _current.popFront();
             if (_current.empty)
             {
-                assert(!_items.empty);
+                assert(!_items.empty, "Attempting to popFront an empty joiner.");
                 _items.popFront();
-                mixin(prepare);
+                mixin(popFrontEmptyElements);
             }
         }
+
+        private enum popFrontEmptyElements = q{
+            // Skip over empty subranges.
+            if (_items.empty) goto end;
+            while (_items.front.empty)
+            {
+                _items.popFront();
+                if (_items.empty) goto end;
+            }
+            // We cannot export .save method unless we ensure subranges are not
+            // consumed when a .save'd copy of ourselves is iterated over. So
+            // we need to .save each subrange we traverse.
+            static if (isForwardRange!RoR && isForwardRange!(ElementType!RoR))
+                _current = _items.front.save;
+            else
+                _current = _items.front;
+        end:
+            assert(1); // required to avoid 'EOF instead of statement' error
+        };
+
         static if (isForwardRange!RoR && isForwardRange!(ElementType!RoR))
         {
             @property auto save()
             {
-                return Result(_items.save, _current.save);
+                auto r = Result(_items.save, _current.save);
+                static if (isBidirectional)
+                    r._currentBack = _currentBack.save;
+                return r;
             }
         }
 
@@ -2573,28 +2582,130 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
                 _current.front = element;
             }
         }
+
+        static if (isBidirectional)
+        {
+            bool checkFinalElement()
+            {
+                import std.range : dropOne;
+
+                if (reachedFinalElement)
+                    return true;
+
+                static if (hasLength!(typeof(_items)))
+                {
+                    if (_items.length == 1)
+                        reachedFinalElement = true;
+                }
+                else
+                {
+                    if (_items.save.dropOne.empty)
+                        reachedFinalElement = true;
+                }
+
+                return false;
+            }
+
+            @property auto ref back()
+            {
+                assert(!empty, "Attempting to fetch the back of an empty joiner.");
+                if (reachedFinalElement)
+                    return _current.back;
+                else
+                    return _currentBack.back;
+            }
+
+            void popBack()
+            {
+                assert(!_current.empty, "Attempting to popBack an empty joiner.");
+                if (checkFinalElement)
+                    _current.popBack();
+                else
+                    _currentBack.popBack();
+
+                bool isEmpty = reachedFinalElement ? _current.empty : _currentBack.empty;
+                if (isEmpty)
+                {
+                    assert(!_items.empty, "Attempting to popBack an empty joiner.");
+                    _items.popBack();
+                    mixin(popBackEmptyElements);
+                }
+            }
+
+            private enum popBackEmptyElements = q{
+                // Skip over empty subranges.
+                if (_items.empty) goto end2;
+                while (_items.back.empty)
+                {
+                    _items.popBack();
+                    if (_items.empty) goto end2;
+                }
+                checkFinalElement;
+                // We cannot export .save method unless we ensure subranges are not
+                // consumed when a .save'd copy of ourselves is iterated over. So
+                // we need to .save each subrange we traverse.
+                static if (isForwardRange!RoR && isForwardRange!(ElementType!RoR))
+                {
+                    if (reachedFinalElement)
+                        _current = _items.back.save;
+                    else
+                        _currentBack = _items.back.save;
+                }
+                else
+                {
+                    if (reachedFinalElement)
+                        _current = _items.back;
+                    else
+                        _currentBack = _items.back;
+                }
+            end2:
+                assert(1);
+            };
+
+            static if (hasAssignableElements!(ElementType!RoR))
+            {
+                @property void back(ElementType!(ElementType!RoR) element)
+                {
+                    assert(!empty, "Attempting to assign to back of an empty joiner.");
+                    if (reachedFinalElement)
+                        _current.back = element;
+                    else
+                        _currentBack.back = element;
+                }
+
+                @property void back(ref ElementType!(ElementType!RoR) element)
+                {
+                    assert(!empty, "Attempting to assign to back of an empty joiner.");
+                    if (reachedFinalElement)
+                        _current.back = element;
+                    else
+                        _currentBack.back = element;
+                }
+            }
+        }
     }
     return Result(r);
 }
 
+///
 @safe unittest
 {
     import std.algorithm.comparison : equal;
-    import std.range.interfaces : inputRangeObject;
     import std.range : repeat;
 
-    static assert(isInputRange!(typeof(joiner([""]))));
+    assert([""].joiner.equal(""));
-    static assert(isForwardRange!(typeof(joiner([""]))));
+    assert(["", ""].joiner.equal(""));
-    assert(equal(joiner([""]), ""));
+    assert(["", "abc"].joiner.equal("abc"));
-    assert(equal(joiner(["", ""]), ""));
+    assert(["abc", ""].joiner.equal("abc"));
-    assert(equal(joiner(["", "abc"]), "abc"));
+    assert(["abc", "def"].joiner.equal("abcdef"));
-    assert(equal(joiner(["abc", ""]), "abc"));
+    assert(["Mary", "has", "a", "little", "lamb"].joiner.equal("Maryhasalittlelamb"));
-    assert(equal(joiner(["abc", "def"]), "abcdef"));
+    assert("abc".repeat(3).joiner.equal("abcabcabc"));
-    assert(equal(joiner(["Mary", "has", "a", "little", "lamb"]),
+}
-                    "Maryhasalittlelamb"));
-    assert(equal(joiner(repeat("abc", 3)), "abcabcabc"));
 
-    // joiner allows in-place mutation!
+/// joiner allows in-place mutation!
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
     auto a = [ [1, 2, 3], [42, 43] ];
     auto j = joiner(a);
     j.front = 44;
@@ -2602,14 +2713,59 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
     assert(equal(j, [44, 2, 3, 42, 43]));
 }
 
+/// insert characters fully lazily into a string
+@safe pure unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : chain, cycle, iota, only, retro, take, zip;
+    import std.format : format;
+
+    static immutable number = "12345678";
+    static immutable delimiter = ",";
+    auto formatted = number.retro
+        .zip(3.iota.cycle.take(number.length))
+        .map!(z => chain(z[0].only, z[1] == 2 ? delimiter : null))
+        .joiner
+        .retro;
+    static immutable expected = "12,345,678";
+    assert(formatted.equal(expected));
+}
+
+@safe unittest
+{
+    import std.range.interfaces : inputRangeObject;
+    static assert(isInputRange!(typeof(joiner([""]))));
+    static assert(isForwardRange!(typeof(joiner([""]))));
+}
+
+@safe unittest
+{
+    // Initial version of PR #6115 caused a compilation failure for
+    // https://github.com/BlackEdder/ggplotd/blob/d4428c08db5ffdc05dfd29690bf7da9073ea1dc5/source/ggplotd/stat.d#L562-L583
+    import std.range : zip;
+    int[] xCoords = [1, 2, 3];
+    int[] yCoords = [4, 5, 6];
+    auto coords = zip(xCoords, xCoords[1..$]).map!( (xr) {
+            return zip(yCoords, yCoords[1..$]).map!( (yr) {
+                    return [
+                    [[xr[0], xr[0], xr[1]],
+                     [yr[0], yr[1], yr[1]]],
+                    [[xr[0], xr[1], xr[1]],
+                     [yr[0], yr[0], yr[1]]]
+                     ];
+            }).joiner;
+    }).joiner;
+}
 
 @system unittest
 {
     import std.algorithm.comparison : equal;
     import std.range.interfaces : inputRangeObject;
+    import std.range : retro;
 
     // bugzilla 8240
     assert(equal(joiner([inputRangeObject("")]), ""));
+    assert(equal(joiner([inputRangeObject("")]).retro, ""));
 
     // issue 8792
     auto b = [[1], [2], [3]];
@@ -2626,6 +2782,109 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
     assert(!equal(js2, js));
 }
 
+/// joiner can be bidirectional
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : retro;
+
+    auto a = [[1, 2, 3], [4, 5]];
+    auto j = a.joiner;
+    j.back = 44;
+    assert(a == [[1, 2, 3], [4, 44]]);
+    assert(equal(j.retro, [44, 4, 3, 2, 1]));
+}
+
+// bidirectional joiner: test for filtering empty elements
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : retro;
+
+    alias El = (e) => new int(e);
+    auto a = [null, [null, El(1), null, El(2), null, El(3), null], null, [null, El(4), null, El(5), null]];
+    auto j = a.joiner;
+
+    alias deref = a => a is null ? -1 : *a;
+    auto expected = [-1, 5, -1, 4, -1, -1, 3, -1, 2, -1, 1, -1];
+    // works with .save.
+    assert(j.save.retro.map!deref.equal(expected));
+    // and without .save
+    assert(j.retro.map!deref.equal(expected));
+    assert(j.retro.map!deref.equal(expected));
+}
+
+// bidirectional joiner is @nogc
+@safe @nogc unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : iota, only, retro;
+
+    auto a = only(iota(1, 4), iota(4, 6));
+    auto j = a.joiner;
+    static immutable expected = [5 , 4, 3, 2, 1];
+    assert(equal(j.retro, expected));
+}
+
+// bidirectional joiner supports assignment to the back
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : popBackN;
+
+    auto a = [[1, 2, 3], [4, 5]];
+    auto j = a.joiner;
+    j.back = 55;
+    assert(a == [[1, 2, 3], [4, 55]]);
+    j.popBackN(2);
+    j.back = 33;
+    assert(a == [[1, 2, 33], [4, 55]]);
+}
+
+// bidirectional joiner works with auto-decoding
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : retro;
+
+    auto a = ["😀😐", "😠"];
+    auto j = a.joiner;
+    assert(j.retro.equal("😠😐😀"));
+}
+
+// test two-side iteration
+@safe unittest
+{
+    import std.algorithm.comparison : equal;
+    import std.range : popBackN;
+
+    auto arrs = [
+        [[1], [2], [3], [4], [5]],
+        [[1], [2, 3, 4], [5]],
+        [[1, 2, 3, 4, 5]],
+    ];
+    foreach (arr; arrs)
+    {
+        auto a = arr.joiner;
+        assert(a.front == 1);
+        assert(a.back == 5);
+        a.popFront;
+        assert(a.front == 2);
+        assert(a.back == 5);
+        a.popBack;
+        assert(a.front == 2);
+        assert(a.back == 4);
+        a.popFront;
+        assert(a.front == 3);
+        assert(a.back == 4);
+        a.popBack;
+        assert(a.front == 3);
+        assert(a.back == 3);
+        a.popBack;
+        assert(a.empty);
+    }
+}
+
 @safe unittest
 {
     import std.algorithm.comparison : equal;
@@ -2745,17 +3004,23 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
         {
             return element;
         }
+        alias back = front;
 
         enum empty = false;
 
-        void popFront()
+        auto save()
         {
+            return this;
         }
 
+        void popFront() {}
+        alias popBack = popFront;
+
         @property void front(int newValue)
         {
             element = newValue;
         }
+        alias back = front;
     }
 
     static assert(isInputRange!AssignableRange);
@@ -2765,17 +3030,30 @@ if (isInputRange!RoR && isInputRange!(ElementType!RoR))
 
     auto range = new AssignableRange();
     assert(range.element == 0);
+    {
+        auto joined = joiner(repeat(range));
+        joined.front = 5;
+        assert(range.element == 5);
+        assert(joined.front == 5);
 
-    auto joined = joiner(repeat(range));
+        joined.popFront;
-    joined.front = 5;
+        int byRef = 7;
-    assert(range.element == 5);
+        joined.front = byRef;
-    assert(joined.front == 5);
+        assert(range.element == byRef);
+        assert(joined.front == byRef);
+    }
+    {
+        auto joined = joiner(repeat(range));
+        joined.back = 5;
+        assert(range.element == 5);
+        assert(joined.back == 5);
 
-    joined.popFront;
+        joined.popBack;
-    int byRef = 7;
+        int byRef = 7;
-    joined.front = byRef;
+        joined.back = byRef;
-    assert(range.element == byRef);
+        assert(range.element == byRef);
-    assert(joined.front == byRef);
+        assert(joined.back == byRef);
+    }
 }
 
 /++