So, I want to get to here:

foreach(x; data.parallel(bucketSize))
{
  // run on worker threads...
}

Phobos alleges to have code for this, but it's not threadsafe, and missing all the pieces that I'm having trouble with.

The TL;DR boils down to this; opApply receives a delegate that is NOT shared.

Problem:

struct S
{
  int opApply(scope int delegate(Iter) shared loopBody) { ... }
}

S s;
foreach(i; s) { ... }

Error: function `S.opApply(scope int delegate(int) shared loopBody)`
is not callable using argument types `(int delegate(int i) pure
nothrow @nogc @safe)`
cannot pass argument `__foreachbody2` of type `int delegate(int i)
pure nothrow @nogc @safe` to parameter `scope int delegate(int) shared
loopBody`

Obviously the loop body is not a shared function... but that's what is required.

So, it seems the arguments to opApply are considered when infering the types for the foreach loop counters, so features of the delegate are being inferred already, but I need the attributes of the delegate to be inferred too, specifically, the `scope` attribute on the delegate needs to be applied to the loop function.

This has cascading issues; the closure contains references to outer objects, but if the delegate is `shared`, then all those references transitively inherit the shared attribute too. It should work exactly like I if it was writing a `shared` method to some struct, and all the members are shared... so the body of the foreach needs to typecheck assuming the transitive application of the inferred attributes from the opApply signature.

As an experiment, I tried this:

void test()
{
        int x;
        void fun() shared
        {
            x++; // <-- error here
        }
        fun();
}

And I got this error:

Error: `shared` function `test.fun` cannot access non-shared data `x`

That looks like a bug. `fun` is a delegate, and it's shared, which means its `this` pointer is shared, which means `this.x` should be shared... but that seems not to be the case.

I think this is a bug. The compile error should have been that I was
unable to call fun() with a non-shared delegate (since the calling
scope is not shared).

I think fixing that issue, combined with inferring the opApply argument attributes onto the foreach body lambda give everything I need to implement a threadsafe foreach.

That was super hard to follow, sorry in advance!

On 14.06.19 03:40, Manu wrote:
> void test()
> {
>          int x;
>          void fun() shared
>          {
>              x++; // <-- error here
>          }
>          fun();
> }
> 
> And I got this error:
> 
> Error: `shared` function `test.fun` cannot access non-shared data `x`
> 
> That looks like a bug.

It's by design. It's not pretty that the qualifiers have a different meaning for member functions and nested functions (because the nested function meaning could be useful for member functions too), but it's what we have.

> `fun` is a delegate, and it's shared, which
> means its `this` pointer is shared, which means `this.x` should be
> shared... but that seems not to be the case.
> ...

There is no `this` pointer. For local functions, `shared` means that every variable accessed in the context is shared.

> I think this is a bug. The compile error should have been that I was
> unable to call fun() with a non-shared delegate (since the calling
> scope is not shared).

There is no way to make the "calling scope shared".

On Thu, Jun 13, 2019 at 7:00 PM Timon Gehr via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 14.06.19 03:40, Manu wrote:
> > void test()
> > {
> >          int x;
> >          void fun() shared
> >          {
> >              x++; // <-- error here
> >          }
> >          fun();
> > }
> >
> > And I got this error:
> >
> > Error: `shared` function `test.fun` cannot access non-shared data `x`
> >
> > That looks like a bug.
>
> It's by design. It's not pretty that the qualifiers have a different meaning for member functions and nested functions (because the nested function meaning could be useful for member functions too), but it's what we have.

Okay. Well, that seems like bad design. Why is it that way? How do we fix this?
I've been working towards parallel foreach for like 18 months here, I
finally have all my ducks in a line, try and type the thing, and it
doesn't work...

> > `fun` is a delegate, and it's shared, which
> > means its `this` pointer is shared, which means `this.x` should be
> > shared... but that seems not to be the case.
> > ...
>
> There is no `this` pointer.

What do you mean? Local functions receive a context just like any method... if it's attributed, then it should be attributed.

> For local functions, `shared` means that
> every variable accessed in the context is shared.

Not clear what you mean. It sounds like you suggest what I expect, but
that's not what I observe.
I expect that if the context is shared, then the transitive rule
applies normal. Everything accessed should be shared...?

> > I think this is a bug. The compile error should have been that I was
> > unable to call fun() with a non-shared delegate (since the calling
> > scope is not shared).
>
> There is no way to make the "calling scope shared".

Yes. So I expect the compile error I suggested?

On Friday, 14 June 2019 at 01:40:46 UTC, Manu wrote:
> This has cascading issues; the closure contains references to outer objects, but if the delegate is `shared`, then all those references transitively inherit the shared attribute too. It should work exactly like I if it was writing a `shared` method to some struct, and all the members are shared... so the body of the foreach needs to typecheck assuming the transitive application of the inferred attributes from the opApply signature.

Data attributes on delegates are not well supported because of https://issues.dlang.org/show_bug.cgi?id=1983

You have to construct the delegate and call the function in the usual way.

> As an experiment, I tried this:
>
> void test()
> {
>         int x;
>         void fun() shared
>         {
>             x++; // <-- error here
>         }
>         fun();
> }
>
> And I got this error:
>
> Error: `shared` function `test.fun` cannot access non-shared data `x`
>
> That looks like a bug. `fun` is a delegate, and it's shared, which means its `this` pointer is shared, which means `this.x` should be shared... but that seems not to be the case.
>
> I think this is a bug. The compile error should have been that I was
> unable to call fun() with a non-shared delegate (since the calling
> scope is not shared).

Unqualified local variable is thread local and shouldn't be accessed by shared closure, because then it would be implicitly shared between threads.

On Fri, Jun 14, 2019 at 1:30 AM Kagamin via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On Friday, 14 June 2019 at 01:40:46 UTC, Manu wrote:
> > This has cascading issues; the closure contains references to outer objects, but if the delegate is `shared`, then all those references transitively inherit the shared attribute too. It should work exactly like I if it was writing a `shared` method to some struct, and all the members are shared... so the body of the foreach needs to typecheck assuming the transitive application of the inferred attributes from the opApply signature.
>
> Data attributes on delegates are not well supported because of https://issues.dlang.org/show_bug.cgi?id=1983
>
> You have to construct the delegate and call the function in the usual way.
>
> > As an experiment, I tried this:
> >
> > void test()
> > {
> >         int x;
> >         void fun() shared
> >         {
> >             x++; // <-- error here
> >         }
> >         fun();
> > }
> >
> > And I got this error:
> >
> > Error: `shared` function `test.fun` cannot access non-shared data `x`
> >
> > That looks like a bug. `fun` is a delegate, and it's shared, which means its `this` pointer is shared, which means `this.x` should be shared... but that seems not to be the case.
> >
> > I think this is a bug. The compile error should have been that
> > I was
> > unable to call fun() with a non-shared delegate (since the
> > calling
> > scope is not shared).
>
> Unqualified local variable is thread local and shouldn't be accessed by shared closure, because then it would be implicitly shared between threads.

Right, exactly... the compile error should be at the assignment of the
not-shared closure to the shared delegate. The error would be
something like "can't assign `Context*` to `shared(Context)*`".
So, the function can certainly exist, but for shared, you should get
an error when you attempt to call it passing the closure to the
function.
const however would work as expected.

So why do I care about a shared local function? Because I can create supporting machinery to assert thread-safety and call it correctly, just like you do to use shared in any way at all. This is an absolutely necessary step that leads to parallel-for loops... one step at a time.

On 14.06.19 07:51, Manu wrote:
> On Thu, Jun 13, 2019 at 7:00 PM Timon Gehr via Digitalmars-d
> <digitalmars-d@puremagic.com> wrote:
>>
>> On 14.06.19 03:40, Manu wrote:
>>> void test()
>>> {
>>>           int x;
>>>           void fun() shared
>>>           {
>>>               x++; // <-- error here
>>>           }
>>>           fun();
>>> }
>>>
>>> And I got this error:
>>>
>>> Error: `shared` function `test.fun` cannot access non-shared data `x`
>>>
>>> That looks like a bug.
>>
>> It's by design. It's not pretty that the qualifiers have a different
>> meaning for member functions and nested functions (because the nested
>> function meaning could be useful for member functions too), but it's
>> what we have.
> 
> Okay. Well, that seems like bad design. Why is it that way? How do we fix this?
> I've been working towards parallel foreach for like 18 months here, I
> finally have all my ducks in a line, try and type the thing, and it
> doesn't work...
> ...

The simplified example above should definitely not compile, and the error message is fine. Your parallel `foreach` should certainly work, and the compiler needs to be fixed to support it. I don't see however what one has to do with the other.

>>> `fun` is a delegate, and it's shared, which
>>> means its `this` pointer is shared, which means `this.x` should be
>>> shared... but that seems not to be the case.
>>> ...
>>
>> There is no `this` pointer.
> 
> What do you mean? Local functions receive a context just like any
> method... if it's attributed, then it should be attributed.
> ...

There is no such thing. This makes no sense. You can't "attribute a context".

>> For local functions, `shared` means that
>> every variable accessed in the context is shared.
> 
> Not clear what you mean. It sounds like you suggest what I expect, but
> that's not what I observe.
> I expect that if the context is shared, then the transitive rule
> applies normal. Everything accessed should be shared...?
> ...

That doesn't mean it is implicitly made shared, it has to be shared to begin with.

>>> I think this is a bug. The compile error should have been that I was
>>> unable to call fun() with a non-shared delegate (since the calling
>>> scope is not shared).
>>
>> There is no way to make the "calling scope shared".
> 
> Yes. So I expect the compile error I suggested?
> 

That error makes no sense. Your problem is that local functions don't infer context qualifiers. E.g.:

void main(){
    pragma(msg, typeof(delegate(){})); // should be: void delegate() immutable pure nothrow @nogc @safe
}

Currently, `immutable` is not included.

This should certainly be fixed. Even if there is not full inference, this should certainly work:

void foo(void delegate()shared dg){}
void main(){ foo((){}); }

On Friday, 14 June 2019 at 09:04:42 UTC, Manu wrote:
> Right, exactly... the compile error should be at the assignment of the
> not-shared closure to the shared delegate. The error would be
> something like "can't assign `Context*` to `shared(Context)*`".
> So, the function can certainly exist, but for shared, you should get
> an error when you attempt to call it passing the closure to the
> function.

The shared function attribute means that its context is shared similar to object methods, the error is an attempt to implicitly leak unshared data into that shared context, that's why indeed function itself is incorrect. Declare the variable as shared and it will work.

On Fri, Jun 14, 2019 at 8:05 AM Kagamin via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On Friday, 14 June 2019 at 09:04:42 UTC, Manu wrote:
> > Right, exactly... the compile error should be at the assignment
> > of the
> > not-shared closure to the shared delegate. The error would be
> > something like "can't assign `Context*` to `shared(Context)*`".
> > So, the function can certainly exist, but for shared, you
> > should get
> > an error when you attempt to call it passing the closure to the
> > function.
>
> The shared function attribute means that its context is shared similar to object methods, the error is an attempt to implicitly leak unshared data into that shared context, that's why indeed function itself is incorrect. Declare the variable as shared and it will work.

No, you've misunderstood. The qualifier does NOT apply to the context
as it should, that's the issue I'm reporting.
Try it with const, it's simpler to understand. You can incorrectly
mutate x from a const context.

A local function is essentially:
void localFun(Context*, Args...);

A const local function should be:
void localFun(const(Context)*, Args...) const  // <- const applies to
the context pointer, as usual.

That does not appear to be the case however.
For example:

void test()
{
  int x;
  void fun() const
  {
    pragma(msg, typeof(x)); // should print `const(int)` because
typeof(const(Context*).x) == const(int), but it incorrectly prints
`int`
    ++x; // <- should be an error, but the context pointer is not
const, so this compiles!
  }
}

The context pointer is missing the qualifier.

Looks like it was an overlooked detail? There are a few messy implementation details with function/delegate. If you take the address of a member function, without a variable. You get a function, from the looks of it that function can be const, which doesn't make sense. But then the fact it is a function and not a delegate also doesn't make sense.

struct S {
   void foo() const {}
}

pragma(msg, typeof(&S.foo)); // void function() const

Anyways even if you do shared like you would in a struct. You can't implicitly convert to shared in the way you can const.

struct D {
    int a;
    void foo() shared {
        pragma(msg, typeof(a)); // int
    }

    void bar() const {
        pragma(msg, typeof(a)); // const int
    }
}

So it is a bug for const at the very least, but shared is ya...

On Fri, Jun 14, 2019 at 1:55 PM Exil via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> Looks like it was an overlooked detail? There are a few messy implementation details with function/delegate. If you take the address of a member function, without a variable. You get a function, from the looks of it that function can be const, which doesn't make sense. But then the fact it is a function and not a delegate also doesn't make sense.
>
> struct S {
>     void foo() const {}
> }
>
> pragma(msg, typeof(&S.foo)); // void function() const
>
> Anyways even if you do shared like you would in a struct. You can't implicitly convert to shared in the way you can const.

Correct, you can not implicitly convert to shared, the error message
should be "Can not convert `Context*` to `shared(Context)*`", or
something to that effect.
That is the error I expect. But *within* the function, the context is
`shared(Context)*`, and that is transitive as usual.
The error should be when making the CALL to the function failing to
perform the implicit conversion of the context.

Where this is interesting for shared, is that I will write the
appropriate machinery to assure a safe calling environment, and then
cast the shared explicitly.
That is how you do any interactions with shared, and it's no different
here; you write unsafe code to confirm the proper environment, and
then cast shared. The library is written to implement and confirm
appropriate context.

> struct D {
>      int a;
>      void foo() shared {
>          pragma(msg, typeof(a)); // int   *** no, it is `shared(int)`
>      }
>
>      void bar() const {
>          pragma(msg, typeof(a)); // const int
>      }
> }
>
> So it is a bug for const at the very least, but shared is ya...

No, in your code above, the function receives a `shared(Context)*`,
and D qualifiers are transitive, so therefore the pragma should print
`shared(int)`, there is absolutely no magic here. Any attempt at magic
here is a terrible bug.
If you try and call foo(), you should get a compile error at the
callsite that it can't pass `Context*` to `shared(Context)*` which the
function receives. So you can't call a shared local function... but
that doesn't mean it's illegal to define one.
Like I say above, the value here is that you can implement machinery
to assert a valid calling environment and force the cast to make the
call.