On 10/18/18 9:35 AM, Steven Schveighoffer wrote:
> 
> struct NotThreadsafe
> {
>    private int x;
>    void local()
>    {
>      ++x; // <- invalidates the method below, you violate the other
> function's `shared` promise
>    }
>    void notThreadsafe() shared
>    {
>      atomicIncrement(&x);
>    }
> }
> 

[snip]

> But on top of that, if I can't implicitly cast mutable to shared, then this ACTUALLY IS thread safe, as long as all the casting in the module is sound (easy to search and verify), and hopefully all the casting is encapsulated in primitives like you have written. Because someone on the outside would have to cast a mutable item into a shared item, and this puts the responsibility on them to make sure it works.
> 

Another thing to point out -- I can make x public (not private), and it's STILL THREAD SAFE.

-Steve

On Thursday, 18 October 2018 at 13:35:22 UTC, Steven Schveighoffer wrote:
> struct ThreadSafe
> {
>    private int x;
>    void increment()
>    {
>       ++x; // I know this is not shared, so no reason to use atomics
>    }
>    void increment() shared
>    {
>       atomicIncrement(&x); // use atomics, to avoid races
>    }
> }

But this isn't thread-safe, for the exact reasons described elsewhere in this thread (and in fact, incorrectly leveled at Manu's proposal). Someone could write this code:

void foo() {
    ThreadSafe* a = new ThreadSafe();
    shareAllOver(a);
    a.increment(); // unsafe, non-shared method call
}

When a.increment() is being called, you have no idea if anyone else is using the shared interface.

This is one of the issues that MP (Manu's Proposal) tries to deal with. Under MP, your code would *not* be considered thread-safe, because the non-shared portion may interfere with the shared portion. You'd need to write two types:

struct ThreadSafe {
    private int x;
    void increment() shared {
        atomicIncrement(&x);
    }
}

struct NotThreadSafe {
    private int x;
    void increment() {
        ++x;
    }
}

These two are different types with different semantics, and forcing them both into the same struct is an abomination.

In your case, the user of your type will need to ensure thread-safety. You may not have any control over how he's doing things, while you *do* control the code in your own type (and module, since that also affects things). Under MP, the type is what needs to be thread-safe, and once it is, the chance of a user mucking things up is much lower.

--
  Simen

On 10/18/18 10:11 AM, Simen Kjærås wrote:
> On Thursday, 18 October 2018 at 13:35:22 UTC, Steven Schveighoffer wrote:
>> struct ThreadSafe
>> {
>>    private int x;
>>    void increment()
>>    {
>>       ++x; // I know this is not shared, so no reason to use atomics
>>    }
>>    void increment() shared
>>    {
>>       atomicIncrement(&x); // use atomics, to avoid races
>>    }
>> }
> 
> But this isn't thread-safe, for the exact reasons described elsewhere in this thread (and in fact, incorrectly leveled at Manu's proposal). Someone could write this code:
> 
> void foo() {
>      ThreadSafe* a = new ThreadSafe();
>      shareAllOver(a);

Error: cannot call function shareAllOver(shared(ThreadSafe) *) with type ThreadSafe *

>      a.increment(); // unsafe, non-shared method call
> }
> 
> When a.increment() is being called, you have no idea if anyone else is using the shared interface.

I do, because unless you have cast the type to shared, I'm certain there is only thread-local aliasing to it.

> This is one of the issues that MP (Manu's Proposal) tries to deal with. Under MP, your code would *not* be considered thread-safe, because the non-shared portion may interfere with the shared portion. You'd need to write two types:
> 
> struct ThreadSafe {
>      private int x;
>      void increment() shared {
>          atomicIncrement(&x);
>      }
> }
> 
> struct NotThreadSafe {
>      private int x;
>      void increment() {
>          ++x;
>      }
> }
> 
> These two are different types with different semantics, and forcing them both into the same struct is an abomination.

Why? What if I wanted to have an object that is local for a while, but then I want it to be shared (and I ensure carefully when I cast to shared that there are no other aliases to that)?

> In your case, the user of your type will need to ensure thread-safety. 

No, the contract the type provides is: if you DON'T cast unshared to shared or vice versa, the type is thread-safe.

If you DO cast unshared to shared, then the type is thread-safe as long as you no longer use the unshared reference.

This is EXACTLY how immutable works.

> You may not have any control over how he's doing things, while you *do* control the code in your own type (and module, since that also affects things). Under MP, the type is what needs to be thread-safe, and once it is, the chance of a user mucking things up is much lower.

Under MP, the type is DEFENSIVELY thread-safe, locking or using atomics unnecessarily when it's thread-local.

-Steve

On Thursday, 18 October 2018 at 13:09:10 UTC, Simen Kjærås wrote:

>>> Well, sorta. But that's not a problem, because you can't do anything that's not threadsafe to something that's shared.
>>
>> Yes you can. You silently agree to another function's assumption that you pass shared data, while actually passing thread-local data and keeping treating it as thread-local. I.e. you silently agree to a race.
>
> No, you don't. If I give you a locked box with no obvious way to open it, I can expect you not to open it.

You contradict yourself and don't even notice it. Per your rules, the way to open that locked box is have shared methods that access data via casting. Also per your rules, there is absolutely no way for the programmer to control whether they're actually sharing the data. Therefore, some API can steal a shared reference without your approval, use that with your "safe" shared methods, while you're continuing to threat your data as not shared.

> It's the same thing. If you have a shared(T), and it doesn't define a thread-safe interface, you can do nothing with it. If you are somehow able to cause a race with something with which you can do nothing, please tell me how, because I'm pretty sure that implies the very laws of logic are invalid.

You and Manu both seem to think that methods allow you to "define a thread-safe interface".

struct S {
    void foo() shared;
}

Per your rules, S.foo is thread-safe. It is here that I remind you, *again*, what S.foo actually looks like, given made-up easy-to-read mangling:

void struct_S_foo(ref shared S);

And yet, for some reason, you think that these are not thread-safe:

void foo(shared int*);
void bar(ref shared int);

I mean, how does that logic even work with you?

>>>> Per your rules, there would be *nothing* in the language to prevent calling S.foo with an unshared Other.
>>>
>>> That's true. And you can't do anything to it, so that's fine.
>>
>> Yes you can do "anything" to it.
>
> No, you can't. You can do thread-safe things to it. That's nothing, *unless* Other defines a shared (thread-safe) interface, in which case it's safe, and everything is fine.
>
> Example:
>
> struct Other {
>     private Data payload;
>
>     // shared function. Thread-safe, can be called from a
>     // shared object, or from an unshared object.
>     void twiddle() shared { payload.doSharedStuff(); }
>
>     // unshared function. Cannot be called from a shared object.
>     // Promises not to interfere with shared data, or to so only
>     // in thread-safe ways (by calling thread-safe methods, or
>     // by taking a mutex or equivalent).
>     void twaddle() { payload.doSharedThings(); }
>
>     // Bad function. Promises not to interfere with shared data,
>     // but does so anyway.
>     // Give the programmer a stern talking-to.
>     void twank() {
>         payload.fuckWith();
>     }
> }
>
> struct S {
>    void foo(shared Other* o) shared {
>        // No can do - can't call non-shared functions on shared object.
>        // o.twaddle();
>
>        // Can do - twiddle is always safe to call.
>        o.twiddle();
>    }
> }

That's already wrong starting at line 2. It should be:

struct Other {
    private shared Data payload; // shared, there's no question about it

    // shared function. Thread-safe, can be called from a
    // shared object, or from an unshared object.
    void twiddle() shared { payload.doSharedStuff(); }

    // unshared function. Cannot be called from a shared object.
    // Promises not to interfere with shared data, or to so only
    // in thread-safe ways (by calling thread-safe methods, or
    // by taking a mutex or equivalent).
    void twaddle() {
        // fine so long as there's a
        // 'auto doSharedThings(ref shared Data)'
        // or an equivalent method for Data.
        // Otherwise it just wouldn't compile, as it should.
        payload.doSharedThings();
    }

    // No longer a bad function, because it doesn't compile, and the
    // programmer can do their own auto-spanking.
    void twank() {
        payload.fuckWith(); // Error: cannot fuckWith() 'shared Data'
    }
}

struct S {
   void foo(shared Other* o) shared {
       // No can do - can't call non-shared functions on shared object.
       // o.twaddle();
       // ^Yep, agreed

       // Can do - twiddle is always safe to call.
       o.twiddle();
   }
}

Well, that was easy, wasn't it?

Your implementation of 'twaddle' is *unsafe*, because the compiler doesn't know that 'payload' is shared. For example, when inlining, it may reorder the calls in it and cause races or other UB. At least one of the reasons behind `shared` *was* to serve as compiler barrier.
What I don't see in your example is where it would be necessary to cast mutable to shared, let alone auto-cast it. And that's the heart of this discussion.

If you just do this:

auto other = new /*shared*/ Other;

...then at the moment, per current rules, you can either twiddle or twaddle (depending on whether you remove the comment or not), but not both, despite being a sole owner of 'Other'. This is the only place where I can see *some small* value in automatic conversion. But I'd much rather have a language that strictly forbids me to do nasty things than provides small conveniences in corner cases.

>> If you couldn't, you wouldn't be able to implement `shared` at all. Forbidding reads and writes isn't enough to guarantee that you "can't do anything with it".
>
> Alright, so I have this shared object that I can't read from, and can't write to. It has no public shared members. What can I do with it? I can pass it to other guys, who also can't do anything with it. Are there other options?

It can have any number of public shared "members" per UFCS. The fact that you forget is that there's no difference between a method and a free function, other than syntax sugar. Well, OK, there's guaranteed private access for methods, but same is true for module members.

>>>> The rest just follows naturally.
>>
>> Nothing follows naturally. The proposal doesn't talk at all about the fact that you can't have "methods" on primitives,
>
> You can't have thread-safe methods operating directly on primitives, because they already present a non-thread-safe interface. This is true. This follows naturally from the rules.

Everything in D already presents a non-threadsafe interface. Things that you advocate included.

struct S {
    void foo() shared;
}

That is not threadsafe. This *sort of* is:

struct S {
    @disable this(this);
    @disable void opAssign(S);

    void foo() shared;
}

...except not quite still. Now, if the compiler generated above in the presence of any `shared` members or methods, then we could begin talking about it being threadsafe. But that part is mysteriously missing from Manu's proposal, even though I keep reminding of this in what feels like every third post or so (I'm probably grossly exaggerating).

>> that you can't distinguish between shared and unshared data if that proposal is realized,

> And you can't do that currently either. Just like today, shared(T) means the T may or may not be shared with other thread. Nothing more, nothing less.

I don't think it means what you think it means. "May or may not be shared with other thread" means "you MUST treat it as if it's shared with other thread". That's it. That's why automatic conversion doesn't make *any* sense, and that's why compiler error on attempting to pass over mutable as shared makes *perfect* sense.

>> that you absolutely destroy D's TLS-by-default treatment...
> I'm unsure what you mean by this.

You lose the ability to distinguish thread-local and shared data.

>>> There's actually one more thing: The one and only thing you can do (without unsafe casting) with a shared object, is call shared methods and free functions on it.
>>
>> Functions that you must not be allowed to write per this same proposal. How quaint.
>
> What? Which functions can't I write?
>
> // Safe, regular function operating on shared data.
> void foo(shared(Other)* o) {
>     o.twiddle(); // Works great!
> }
> // Unsafe function. Should belong somewhere deep in druntime
> // and only be used by certified wizards.
> void bar(shared(int)* i) {
>     atomicOp!"++"(i);
> }

Uh-huh, only due to some weird convention that "methods" are somehow safer than free functions. Which they're not.

>>>> 1. Primitive types can't be explicitly `shared`.
>>>
>>> Sure they can, they just can't present a thread-safe interface, so you can't do anything with a shared(int).
>>
>> Ergo... you can't have functions taking pointers to shared primitives. Ergo, `shared <primitive type>` becomes a useless language construct.
>
> Yup, this is correct. But wrap it in a struct, like e.g. Atomic!int, and everything's hunky-dory.

So again,

void atomicInc(shared int*); // is "not safe", but
void struct_Atomic_int_opUnary_plus_plus(ref shared Atomic); // is "safe"

just because latter is a "method". And that, by you, is hunky-dory? Whether it's a method or a free function, it's written to work on *shared* data. Of course it wouldn't be safe if you allow any non-shared data to become shared without the programmer having a say in this.

> I have no idea where I or Manu have said you can't make functions that take shared(T)*.

Because you keep saying they're unsafe and that you should wrap them up in a struct for no other reason than just "because methods are kosher".

>> I sort of expected that answer. No, nothing is implicitly const. When you pass a reference to a function taking const, *you keep mutable reference*, the function agrees to that, and it's only "promise" is to not modify data through the reference you gave it. But *you still keep mutable reference*. Just as you would keep *unshared mutable* reference if implicit conversion from mutable to shared existed.
>
> Yup, and that's perfectly fine, because 'shared' means 'thread-safe'. I think Manu might have mentioned that once.
>
> If a type presents both a shared and a non-shared interface, and the non-shared interface may do things that impact the shared part, these things must be done in a thread-safe manner.
> If that's not the case, you have a bug. The onus is on the creator of a type to do this.

Yes, that part is perfectly fine with me.

> Let's say it together: for a type to be thread-safe, all of its public members must be written in a thread-safe way.

It's shared private parts also must be written in a thread-safe way. Yes, they're private, but they still may be shared. Welcome to the communism of multithreading.

> If a non-shared method may jeopardize this, the type is not thread-safe, and shouldn't provide a shared interface.

It can't jeopardize anything so long as you actually treat your shared data accordingly, and don't just magically assume unshared parts to be shared for some reason.

On Thursday, 18 October 2018 at 16:31:02 UTC, Stanislav Blinov wrote:
> So again,
>
> void atomicInc(shared int*); // is "not safe", but
> void struct_Atomic_int_opUnary_plus_plus(ref shared Atomic); // is "safe"
>
> just because latter is a "method". And that, by you, is hunky-dory? Whether it's a method or a free function, it's written to work on *shared* data. Of course it wouldn't be safe if you allow any non-shared data to become shared without the programmer having a say in this.

Out of curiosity, when it comes to primitives, what could you do under MP in void "atomicInc(shared int*)" that would be problematic?

void atomicInc(shared int*) {
  // i.e. what goes here?
}

On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.

How is this significantly different from now?

-----------------
shared int i;
++i;

Error: read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(i, 1) instead.
-----------------

There's not much one can do to modify a shared value as it is.

> Current situation where you can arbitrarily access shared members
> undermines any value it has.

Unless I'm missing something, I can't arbitrarily do anything with shared members right now.

> Shared must assure you don't access
> members unsafely, and the only way to do that with respect to data
> members, is to inhibit access completely.

Or use atomic operations.

> Assuming this world... how do you use shared?

https://github.com/atilaneves/fearless

or

https://dlang.org/phobos/core_atomic.html

> From there, it opens up another critical opportunity; T* -> shared(T)*
> promotion.

I don't think that works. See below.

> If you write a lock-free queue for instance, and all the methods are
> `shared` (ie, threadsafe), then under the current rules, you can't
> interact with the object when it's not shared, and that's fairly
> useless.

Not really:

-----------
struct FancyQueue(E) {
    // ...
    void pushBack(this T)(E element) {
        static if(is(T == shared)) {
            // lock mutex or whatever is needed
            auto safeThis = () @trusted { return cast(FancyQueue) this; }();
        } else {
            // no need to lock anything
            alias safeThis = this;
        }
        // profit
    }
}
-----------

Usable if shared or not.

> Assuming the rules above: "can't read or write to members", and the understanding that `shared` methods are expected to have threadsafe implementations (because that's the whole point), what are the risks from allowing T* -> shared(T)* conversion?

int i;
tid.send(&i);
++i;  // oops, data race


> All the risks that I think have been identified previously assume that you can arbitrarily modify the data.

Do you have any examples of arbitrarily modifying shared data? I can't think of any.

> That's  insanity... assume we fix that... I think the promotion actually becomes safe now...?

I don't think so, no.

On Thursday, 18 October 2018 at 17:10:03 UTC, aliak wrote:

> Out of curiosity, when it comes to primitives, what could you do under MP in void "atomicInc(shared int*)" that would be problematic?
>
> void atomicInc(shared int*) {
>   // i.e. what goes here?
> }

1. Anything if int* implicitly converts to shared int* (per MP), because then that function is indeed unsafe.
2. Only actual platform-specific implementation bugs otherwise, and these are beyond what `shared` can provide.

On 10/18/18 1:17 PM, Atila Neves wrote:
> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.
> 
> How is this significantly different from now?
> 
> -----------------
> shared int i;
> ++i;

 i = i + 1; // OK(!)

-Steve

On Thursday, 18 October 2018 at 17:43:40 UTC, Steven Schveighoffer wrote:
> On 10/18/18 1:17 PM, Atila Neves wrote:
>> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>>> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.
>> 
>> How is this significantly different from now?
>> 
>> -----------------
>> shared int i;
>> ++i;
>
>  i = i + 1; // OK(!)
>
> -Steve

Sigh... I'm sure there's a practical reason for it, but still.

On Thursday, 18 October 2018 at 17:17:37 UTC, Atila Neves wrote:
> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.
>
> How is this significantly different from now?
>
> -----------------
> shared int i;
> ++i;
>
> Error: read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(i, 1) instead.
> -----------------
>
> There's not much one can do to modify a shared value as it is.

i = 1;
int x = i;
shared int y = i;

And so on. The compiler needs to forbid this.

> Unless I'm missing something, I can't arbitrarily do anything with shared members right now.

Except arbitrarily read and write them :)

>> From there, it opens up another critical opportunity; T* -> shared(T)*
>> promotion.

> I don't think that works. See below.

Welcome to the club.

>> Assuming the rules above: "can't read or write to members", and the understanding that `shared` methods are expected to have threadsafe implementations (because that's the whole point), what are the risks from allowing T* -> shared(T)* conversion?
>
> int i;
> tid.send(&i);
> ++i;  // oops, data race

Doesn't work. No matter what you show Manu or Simen here they think it's just a bad contrived example. You can't sway them by the fact that the compiler currently *prevents* this from happening.

>> All the risks that I think have been identified previously assume that you can arbitrarily modify the data.

> Do you have any examples of arbitrarily modifying shared data? I can't think of any.

See near the beginning of this post ;)

>> That's  insanity... assume we fix that... I think the promotion actually becomes safe now...?
>
> I don't think so, no.

+100500.