Never mind its already in.

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

On 19.06.19 11:13, Walter Bright wrote:
> 
> Using the resulting value of x:
> 
>    int x;
>    void fun(scope ref shared(int) x) { ... }
>    fun(x); // implicit promotion to shared in this case
>    int y = x; // add this line
> 
> And now there's a problem. The compiler thinks x is thread local, so it doesn't add synchronization to the read of x.

Probably `fun` should be `immutable` or `shared` as well. Otherwise there is no way to tell at the call site that it does not also capture `x` mutably, in which case the implicit promotion couldn't be allowed.

> Not adding synchronization means that although fun() has terminated all the threads it spawned accessing x, it does not mean the memory caches of x are updated.
> ...

The @trusted code that decided to send the `scope` reference to other threads has to make sure that all reads/writes to the `scope`d reference context are visible on the current thread before it returns. (Otherwise, from the perspective of the current thread, there are still copies of the `scope` reference around, even though the function call has terminated, which goes against `scope` guarantees.)

> While you may have coded fun() to update the caches of x before returning, the compiler can't know that, and so the compiler cannot allow the implicit conversion.
> ...

The compiler can allow the implicit conversion because in @safe code there is no way to send `scope` data to other threads and @trusted code has to respect `scope` guarantees.

> In other words, adding `scope` does not guarantee SC-DRF (Sequential Consistency - Data Race Free).

I think it does. The challenge is to ensure that the implicit promotion to `shared` doesn't result in `shared`/unshared aliasing. I don't think there is a problem with implicitly casting back to unshared, as the called function promises not to leak the references.

On 19.06.19 05:11, Manu wrote:
> On Wed, Jun 19, 2019 at 12:20 AM Timon Gehr via Digitalmars-d
> <digitalmars-d@puremagic.com> wrote:
>>
>> On 18.06.19 16:14, Timon Gehr wrote:
>>> On 18.06.19 16:13, Timon Gehr wrote:
>>>>
>>>> int sum(){ // note: just to illustrate the concept
>>>>       int result=0;
>>>>       foreach(i;iota(1000).parallel){
>>>>           static assert(is(typeof(result)==shared(int)));
>>>>           result.atomic!"+="(i);
>>>>       }
>>>>       static assert(is(typeof(result)==int));
>>>>       return result;
>>>> }
>>>
>>> Return type should have been int, of course...
>>
>> And there should have been is expressions. Getting tired from pushing
>> back against all the nonsense. Just note that if you can make the above
>> work with useless qualified capturing, you can do so with useful
>> qualified capturing and this is so blatantly obvious that it causes me
>> physical pain that Manu honestly does not see it.
> 
> I can't see it because the design specifically inhibits that static
> assert in the loop body.

Ok. I guess it suffices to say that it does not. (If you can promote the entire stack frame to `shared` temporarily, you can just as easily promote only the part of the stack frame that your delegate actually wants to capture.)

> But I will revoke my opinion on this matter; arguing will only slow it
> down. If you know how to make that *EXACT* code you wrote above work,

I don't know yet what the _most general_ implicit `shared` promotion rules could be, but I think I know how to do it in the type system for your specific case.

For memory ordering, we can just say that because there is `scope` on the delegate, any @trusted code that sends it to other threads temporarily is clearly already required to set up the respective memory barriers. (Because it needs to ensure that the deletion of all references to the closure context in all other threads is ordered before the owning thread returns.)

> then we are done here and I will buy you a years supply of beer.
> ...

I don't drink. :)

> Coupled with removing read/write access from shared, that is
> everything I need to go away and leave you all alone.
> 

Then we are on the same page, but this will need a DIP, so it would make sense to think about it a bit more and to convince Walter that it can work.

On 6/19/2019 6:45 AM, Timon Gehr wrote:
> On 19.06.19 11:13, Walter Bright wrote:
>>
>> Using the resulting value of x:
>>
>>    int x;
>>    void fun(scope ref shared(int) x) { ... }
>>    fun(x); // implicit promotion to shared in this case
>>    int y = x; // add this line
>>
>> And now there's a problem. The compiler thinks x is thread local, so it doesn't add synchronization to the read of x.
> 
> Probably `fun` should be `immutable` or `shared` as well. Otherwise there is no way to tell at the call site that it does not also capture `x` mutably, in which case the implicit promotion couldn't be allowed.

`scope` prevents capture of a reference to `x`, mutable or not, that persists after the return from `fun`.


>> Not adding synchronization means that although fun() has terminated all the threads it spawned accessing x, it does not mean the memory caches of x are updated.
>> ...
> 
> The @trusted code that decided to send the `scope` reference to other threads has to make sure that all reads/writes to the `scope`d reference context are visible on the current thread before it returns. (Otherwise, from the perspective of the current thread, there are still copies of the `scope` reference around, even though the function call has terminated, which goes against `scope` guarantees.)

The way atomic synchronization of variables works is when a write is performed, a write access fence happens after. When a read access is performed, a read access fence is performed.

The last write to shared x in fun will do a write access fence, but the read access fence won't happen until there's the read of shared x, the read access fence won't happen. But the x after the return of fun is not shared, no read access fence will happen, and the read may not see the results of the last write to shared x.


>> While you may have coded fun() to update the caches of x before returning, the compiler can't know that, and so the compiler cannot allow the implicit conversion.
>> ...
> 
> The compiler can allow the implicit conversion because in @safe code there is no way to send `scope` data to other threads and @trusted code has to respect `scope` guarantees.

The scope guarantees are NOT sequential consistency data-race-free guarantees for multithreaded access.


>> In other words, adding `scope` does not guarantee SC-DRF (Sequential Consistency - Data Race Free).
> I think it does. The challenge is to ensure that the implicit promotion to `shared` doesn't result in `shared`/unshared aliasing. I don't think there is a problem with implicitly casting back to unshared, as the called function promises not to leak the references.

It does not. A write to an atomic is:

    write variable
    read fence

A read from an atomic is:

    read fence
    read variable

SC-DRF of atomics absolutely depends on this. The example code does:

  thread 1:
    write x
    read fence

  thread 2:
    read x

This is wrong wrong wrong from what I know about atomics.

The only way a local reference can be implicitly converted to a shared reference is if the compiler can prove there are no other local references to that memory location, which is essentially what Rust does. The read of `x` after `fun` returns violates that principle, and the result is a data race.

---

There's another way to look at this:

1. When a politician promises a Free Lunch, you're gonna pay for it.
2. When a scientist discovers perpetual motion, he's made a mistake.
3. When a salesman sells you an effortless exercise machine, you won't get stronger.
4. When you respond to an ad for a "Get Rich Quick Through Real Estate" seminar, you're a sucker.

  -- and --

5. When a data-race-free solution is found that doesn't follow fencing protocols, it doesn't work.

On 19.06.19 23:03, Walter Bright wrote:
> On 6/19/2019 6:45 AM, Timon Gehr wrote:
>> On 19.06.19 11:13, Walter Bright wrote:
>>>
>>> Using the resulting value of x:
>>>
>>>    int x;
>>>    void fun(scope ref shared(int) x) { ... }
>>>    fun(x); // implicit promotion to shared in this case
>>>    int y = x; // add this line
>>>
>>> And now there's a problem. The compiler thinks x is thread local, so it doesn't add synchronization to the read of x.
>>
>> Probably `fun` should be `immutable` or `shared` as well. Otherwise there is no way to tell at the call site that it does not also capture `x` mutably, in which case the implicit promotion couldn't be allowed.
> 
> `scope` prevents capture of a reference to `x`, mutable or not, that persists after the return from `fun`.
> ...

I was talking about this:

int x0;
void fun(scope ref shared(int) x1){
    assert(&x0 is &x1); //uh, oh
    // ...
}
fun(x);

> ...
>>
>> The compiler can allow the implicit conversion because in @safe code there is no way to send `scope` data to other threads and @trusted code has to respect `scope` guarantees.
> 
> The scope guarantees are NOT sequential consistency data-race-free guarantees for multithreaded access.
> ...

Right now, in `@safe` code it guarantees that there is _no_ multithreaded access, no? How do you leak a `scope` reference to another thread? If there is _no_ multithreaded access, you don't get any data races, so `scope` guarantees no data races on the `scope`d memory location given that the reference wasn't accessible from multiple threads before.

> 
>>> In other words, adding `scope` does not guarantee SC-DRF (Sequential Consistency - Data Race Free).
>> I think it does. The challenge is to ensure that the implicit promotion to `shared` doesn't result in `shared`/unshared aliasing. I don't think there is a problem with implicitly casting back to unshared, as the called function promises not to leak the references.
> 
> It does not. A write to an atomic is:
> 
>      write variable
>      read fence
> 
> A read from an atomic is:
> 
>      read fence
>      read variable
> 
> SC-DRF of atomics absolutely depends on this. The example code does:
> 
>    thread 1:
>      write x
>      read fence
> 
>    thread 2:
>      read x
> 
> This is wrong wrong wrong from what I know about atomics.
> ...

Yes, what's above certainly does not work. My point was that that is not what the code would do, at least not legally. If the above happens, there would be some `@trusted` code somewhere that you can blame for it.

Sending a `scope` variable to another thread is a `@system` operation. In order to ensure that `scope` guarantees are met, the `@trusted` code that performs the send needs to add memory barriers that ensure that all writes to the `scope` variable are visible on the current thread before it returns. So on thread 2 you would actually also have barriers before `x` is accessed again.

Let's consider a case _without_ implicit promotion:

shared(int) x=0; // x is shared all the way
static void foo(scope ref shared(int) x)@trusted{
    // possibly send &x to other threads that mutate it and
    // then forget about &x
    // ...
}
foo(x);
int y=x; // read x
enforce(x==y); // read x again

My claim is that if the enforcement fails, this indicates a bug in `foo`, because it violates guarantees you can derive from the fact that `x` is a local variable that is only passed to some other code via a `scope`d parameter: when `x` is read for the first time, we know that actually no other thread can have a reference to it which it can use for a write, so its value has to remain constant.

It should therefore be possible to read `x` without any further barriers after `foo` returns. This is because `foo` already needs to ensure all writes from other threads are visible.

It is possible that I am missing something (even though what you wrote is not it). Is there a possible implementation of `foo` that would be correct (in a language that does not have any implicit `shared` promotion), but for which a data race on `x` would result if we didn't add another fence before reading `x` after `foo` has returned?

> 
> ...
> 
> 5. When a data-race-free solution is found that doesn't follow fencing protocols, it doesn't work.

We are on the same page on this. :)

On 20.06.19 00:04, Timon Gehr wrote:
> 
> int x0;
> void fun(scope ref shared(int) x1){
>      assert(&x0 is &x1); //uh, oh
>      // ...
> }
> fun(x0);

(Sorry, was `fun(x)` instead of `fun(x0)`.)

On 6/19/2019 3:04 PM, Timon Gehr wrote:
> I was talking about this:
> 
> int x0;
> void fun(scope ref shared(int) x1){
>      assert(&x0 is &x1); //uh, oh
>      // ...
> }
> fun(x);

I see.


> Right now, in `@safe` code it guarantees that there is _no_ multithreaded access, no? How do you leak a `scope` reference to another thread? If there is _no_ multithreaded access, you don't get any data races, so `scope` guarantees no data races on the `scope`d memory location given that the reference wasn't accessible from multiple threads before.

`scope` only guarantees no references to the scoped variable exist following the exit of the function. What happens inside is anything goes as long as that invariant is maintained. That would include starting up a new thread that can access the scope variable, as long as the thread terminates its use of that variable before the function exits.


> Yes, what's above certainly does not work. My point was that that is not what the code would do, at least not legally. If the above happens, there would be some `@trusted` code somewhere that you can blame for it.
> 
> Sending a `scope` variable to another thread is a `@system` operation. In order to ensure that `scope` guarantees are met, the `@trusted` code that performs the send needs to add memory barriers that ensure that all writes to the `scope` variable are visible on the current thread before it returns. So on thread 2 you would actually also have barriers before `x` is accessed again.

`scope` does not offer such guarantees. It only guarantees no leaks of the reference that survive the end of the function. It does not guarantee synchronization of memory caches.

> 
> Let's consider a case _without_ implicit promotion:
> 
> shared(int) x=0; // x is shared all the way
> static void foo(scope ref shared(int) x)@trusted{
>      // possibly send &x to other threads that mutate it and
>      // then forget about &x
>      // ...
> }
> foo(x);
> int y=x; // read x
> enforce(x==y); // read x again
> 
> My claim is that if the enforcement fails, this indicates a bug in `foo`, because it violates guarantees you can derive from the fact that `x` is a local variable that is only passed to some other code via a `scope`d parameter: when `x` is read for the first time, we know that actually no other thread can have a reference to it which it can use for a write, so its value has to remain constant.

That is not what `scope` does, but the example is still correct because the language does not say that x can be converted to a local, and so it is not converted, and the memory synchronization of x is maintained and it works.


> It should therefore be possible to read `x` without any further barriers after `foo` returns. This is because `foo` already needs to ensure all writes from other threads are visible.

`scope` does not add such barriers. The implementer of foo may add a fence to do that, but the compiler doesn't know that and cannot rely on it.


> It is possible that I am missing something (even though what you wrote is not it). Is there a possible implementation of `foo` that would be correct (in a language that does not have any implicit `shared` promotion), but for which a data race on `x` would result if we didn't add another fence before reading `x` after `foo` has returned?

AFAIK, another fence is required upon return of foo and before reading x. It's not just about whether an extra reference exists, it's about when the memory cache coherency happens.

>> 5. When a data-race-free solution is found that doesn't follow fencing protocols, it doesn't work.
> 
> We are on the same page on this. :)

Phew! Good!

On Wed, Jun 19, 2019 at 7:15 PM Walter Bright via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 6/17/2019 4:46 PM, Manu wrote:
> > Is this valid?
> >
> > int x;
> > void fun(scope ref shared(int) x) { ... }
> > fun(x); // implicit promotion to shared in this case
> >
> > This appears to promote a thread-local to shared. The problem with such promotion is that it's not valid that a thread-local AND a shared reference to the same thing can exist at the same time.
> >
> > With scope, we can guarantee that the reference doesn't escape the callee.
> > Since the argument is local to the calling thread, and since the
> > calling thread can not be running other code at the same time as the
> > call is executing, there is no way for any code to execute with a
> > thread-local assumption while the callee makes shared assumptions.
> >
> > I think this might be safe?
> >
>
> Using the resulting value of x:
>
>    int x;
>    void fun(scope ref shared(int) x) { ... }
>    fun(x); // implicit promotion to shared in this case
>    int y = x; // add this line
>
> And now there's a problem. The compiler thinks x is thread local, so it doesn't add synchronization to the read of x. Not adding synchronization means that although fun() has terminated all the threads it spawned accessing x, it does not mean the memory caches of x are updated.
>
> While you may have coded fun() to update the caches of x before returning, the compiler can't know that, and so the compiler cannot allow the implicit conversion.
>
> In other words, adding `scope` does not guarantee SC-DRF (Sequential Consistency
> - Data Race Free).

Well, you're assuming that `fun()` is an unsafe function that did violate the scope agreement and passed it out to worker threads. *ANY* deployment of @trusted code requires that you know what you're doing. So, rather, let's assume that fun() does NOT do that, and it is in fact @safe... your concern does not arise in that world where you're not manually violating the type system.

So the issue you describe is only possible where you have deliberately
written unsafe code to perform a very low-level operation. You're
obviously incompetent if you don't implement the sequential
consistency machinery at the conclusion of your low-level machine.
I don't think saying "this person wasn't qualified to implement a
@trusted function" is a valid reason to reject an extremely useful
improvement.

TL;DR: I don't think your argument is valid. If it were, you would have to apply that reasoning to every @trusted function out there. Language features are not required to consider that a @trusted author may make a mistake; that is literally the point of @trusted.

On 6/19/2019 11:53 PM, Manu wrote:
> Well, you're assuming that `fun()` is an unsafe function that did
> violate the scope agreement and passed it out to worker threads.

Not at all. Scope means it does not leak a reference that survives the end of the function. As long as the threads terminate before the function returns, it is valid code.

On Fri, Jun 21, 2019 at 5:55 PM Walter Bright via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 6/19/2019 11:53 PM, Manu wrote:
> > Well, you're assuming that `fun()` is an unsafe function that did violate the scope agreement and passed it out to worker threads.
>
> Not at all. Scope means it does not leak a reference that survives the end of the function. As long as the threads terminate before the function returns, it is valid code.

I agree it's 'theoretically' valid; that's why I suggested it and the
construct makes sense.
How would you write code that escapes the scope reference to another
thread? This must require some @trusted code to achieve sharing
between threads; at that point, the @trusted function is responsible
for re-instating the thread-locality guarantees (which are trivial to
implement). If you don't do @trusted mischief, there's no way to
migrate the value across threads, so your synchronisation issue
shouldn't exist if no @trusted code exists.