Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generator implements InputRange #5194

Closed
wants to merge 4 commits into from
Closed

Generator implements InputRange #5194

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee4778e
Since std.concurrency.Generator is a class already, I made it to impl…
dukc Feb 24, 2017
cfb25fb
Fix for style violation.
dukc Feb 27, 2017
1f926a1
Removed a reduntant import.
dukc Feb 27, 2017
49bd11b
Unittest now covers indexed foreach.
dukc Mar 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 72 additions & 2 deletions std/concurrency.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,8 +1586,9 @@ private interface IsGenerator {}
* }
* ---
*/
import std.range.interfaces : InputRange;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goes at the top of the file

Copy link
Contributor Author

@dukc dukc Feb 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't: there was one already! I removed mine altogether.

EDIT: I read that import wrong. Went messy so i squashed.

class Generator(T) :
Fiber, IsGenerator
Fiber, IsGenerator, InputRange!T
{
/**
* Initializes a generator object which is associated with a static
Expand Down Expand Up @@ -1682,13 +1683,54 @@ class Generator(T) :


/**
* Returns the most recently generated value.
* Returns the most recently generated value by shallow copy.
*/
final T front() @property
{
return *m_value;
}

/**
* Returns the most recently generated value without excuting a copy
* contructor. Will not compile for element types defining a
* postblit, because Generator does not return by reference.
*/
final T moveFront()
{
import std.algorithm, std.range;
static if (!hasElaborateCopyConstructor!T)
{
return front;
}
else
{
static assert(0,
"Fiber front is rvalue and thus cannot be moved when it defines a postblit.");
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different to the generic moveFront implementation?

    static if (is(typeof(&r.moveFront)))
    {
        return r.moveFront();
    }
    else static if (!hasElaborateCopyConstructor!(ElementType!R))
    {
        return r.front;
    }
    else static if (is(typeof(&(r.front())) == ElementType!R*))
    {
        import std.algorithm.mutation : move;
        return move(r.front);
    }
    else
    {
        static assert(0,
                "Cannot move front of a range with a postblit and an rvalue front.");
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the message, no difference. I tried to call it, but the first static if caused infinite recursion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me:

https://dpaste.dzfl.pl/de6644a70448

Copy link
Contributor Author

@dukc dukc Mar 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, we are talking about std.concurrency.Generator, not about std.range.generate. But that code would work with Generator too, in DPaste.

The reason it does not work in Generator: Look the first static if block in moveFront implementation you just posted. MoveFront(generator) calls generator.moveFront if it can find one -the very function I'm defining!


final int opApply(scope int delegate(T) loopBody)
{
int broken;
for(; !empty; popFront())
{
broken = loopBody(front);
if(broken) break;
}
return broken;
}

final int opApply(scope int delegate(size_t, T) loopBody)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if we have to add opApply to everything that is an InputRange the range concept of "building block" would have somehow failed. Do you know about enumerate and why doesn't it work for you?

Copy link
Contributor Author

@dukc dukc Feb 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not think about it. But wouldn't that plant a land mine for maintenance? If somebody adds an opApply implementation using foreach to enumerate(), it would regress Generators opApply because it would recurse infinitely then.

Copy link
Contributor Author

@dukc dukc Feb 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could add to avoid code duplication for cases like this, is a template mixin version of opApply() and moveFront().

Edit: Or a generic range template which guarantees in it's spec to forward only primitives and nothing else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If somebody adds an opApply implementation using foreach to enumerate(), it would regress Generators opApply because it would recurse infinitely then.

I am sorry, but I can't follow. How would sth. like this half-pseudo code end in an infinite recursion?

struct Enumerate(R)
{
    R r;
    int opApply(scope int delegate(size_t, ElementType!R) dg)
    {
         int err;
         auto r2 = r.save;
         for (size_t i; !r2.empty; r2.popFront(), i++)
         {
             err = dg(i, r2.front);
             if (err) return err;
         }
         return 0;
    }
}
myGenerator.enumerate.array;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also, e.g. https://dpaste.dzfl.pl/57536541877a

Copy link
Contributor Author

@dukc dukc Mar 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That code won't. But if it's done like (untested, may have errors):

int opApply(scope int delegate(size_t, ElementType!R) dg)
{
         int err;
        static if(__traits(compiles, foreach(i,e;r){})) foreach (i, e; r.save)
        {
              err = dg(i, e);
             if (err) return err;
         } else for (size_t i; !r2.empty; r2.popFront(), i++)
         {
             auto r2 = r.save;
             err = dg(i, r2.front);
             if (err) return err;
          }
         return 0;
}

That, I think, will, because the foreach inside calls the very indexed opApply of R what's using the Enumerate in it's implementation.

{
int broken;
for(size_t i; !empty; ++i, popFront())
{
broken = loopBody(i, front);
if(broken) break;
}
return broken;
}


private:
T* m_value;
Expand Down Expand Up @@ -1724,6 +1766,7 @@ void yield(T)(T value)
{
import core.exception;
import std.exception;
import std.range.interfaces;

static void testScheduler(Scheduler s)
{
Expand Down Expand Up @@ -1765,14 +1808,41 @@ void yield(T)(T value)
{
tid.send(e);
}

});
scheduler = null;
}

testScheduler(new ThreadScheduler);
testScheduler(new FiberScheduler);
}
///
@system unittest
{
import std.range;

InputRange!int myIota = iota(10).inputRangeObject;

myIota.popFront();
myIota.popFront();
assert(myIota.moveFront == 2);
assert(myIota.front == 2);
myIota.popFront();
assert(myIota.front == 3);

//can be assigned to std.range.interfaces.InputRange directly
myIota = new Generator!int(
{
foreach(i; 0 .. 10) yield(i);
});

myIota.popFront();
myIota.popFront();
assert(myIota.moveFront == 2);
assert(myIota.front == 2);
myIota.popFront();
assert(myIota.front == 3);
}

// MessageBox Implementation

Expand Down