shared - i need it to be useful

[Stanislav Blinov]

On Tuesday, 16 October 2018 at 00:15:54 UTC, Manu wrote:
> On Mon, Oct 15, 2018 at 4:35 PM Stanislav Blinov via 
> Digitalmars-d <digitalmars-d at puremagic.com> wrote:

>> What?!? So... my unshared methods should also perform all 
>> that's necessary for `shared` methods?

> Of course! You're writing a threadsafe object... how could you 
> expect otherwise?

See below.

> Just to be clear, what I'm suggesting is a significant 
> *restriction*
> to what shared already does... there will be a whole lot more 
> safety under my proposal.

I don't see how an *implicit* cast can be a restriction. At all.

> The cast gives exactly nothing that attributing a method as 
> shared
> doesn't give you, except that attributing a method shared is so 
> much
> more sanitary and clearly communicates intent at the API level.

It's like we're talking about wholly different things here. 
Casting should be done by the caller, i.e. a programmer that uses 
some API. If that API expects shared arguments, the caller better 
make sure they pass shared values. Implicit conversion destroys 
any obligations between the caller and the API.

>> > You can write bad code with any feature in any number of 
>> > ways.
>>
>> Yup. For example, passing an int* to a function expecting 
>> shared int*.
>
> I don't understand your example. What's the problem you're 
> suggesting?

The problem that I'm suggesting is exactly that: an `int*` is 
not, and can not, be a `shared int*` at the same time. Substitute 
int for any type. But D is not Rust and it can't statically 
prevent that, except for disallowing trivial programming 
mistakes, which, with implicit conversion introduced, would also 
go away.

>> ...And therefore they lack any synchronization. So I don't see 
>> how they *can* be "compatible" with `shared` methods.
>
> I don't understand this statement either. Who said they lack 
> synchronisation? If they need it, they will have it. There's a 
> good chance they don't need it though, they might not  interact 
> with a thread-unsafe portion of the class.

Or they might.

>> > If your shared method is incompatible with other methods, 
>> > your class is broken, and you violate your promise.
>>
>> Nope.
>
> So certain...
>
>> class BigCounter {
>>
>>      this() { /* don't even need the mutex if I'm not sharing 
>> this
>> */ }
>>
>>      this(Mutex m = null) shared {
>>          this.m = m ? m : new Mutex;
>>      }
>>
>>      void increment() { value += 1; }
>>      void increment() shared { synchronized(m)
>> *value.assumeUnshared += 1; }
>>
>> private:
>>      Mutex m;
>>      BigInt value;
>> }
>
> You've just conflated 2 classes into one. One is a threadlocal
> counter, the other is a threadsafe counter. Which is it?
> Like I said before: "you can contrive a bad program with 
> literally any language feature!"

Because that is exactly the code that a good amount of 
"developers" will write. Especially those of the "don't think 
about it" variety. Don't be mistaken for a second: if the 
language allows it, they'll write it.

>> They're not "compatible" in any shape or form.
>
> Correct, you wrote 2 different things and mashed them together.

Can you actually provide an example of a mixed shared/unshared 
class that even makes sense then? As I said, at this point I'd 
rather see such definitions prohibited entirely.

>> Or would you have
>> the unshared ctor also create the mutex and unshared increment 
>> also take the lock? What's the point of having them  then? 
>> Better disallow mixed implementations altogether (which is 
>> actually not that bad of an idea).

> Right. This is key to my whole suggestion. If you write a 
> shared thing, you accept that it's shared! You don't just 
> accept it, you jam the stake in the ground.

Then, once more, `shared` should then just be a type qualifier 
exclusively, and mixing shared/unshared methods should just not 
be allowed.

> There's a relatively small number of things that need to be
> threadsafe, you won't see `shared` methods appearing at random. 
> If you use shared, you promise threadsafety OR the members of 
> the thing are inaccessible without some sort of 
> lock-&-cast-away treatment.

As above.

>> import std.concurrency;
>> import core.atomic;
>>
>> void thread(shared int* x) {
>>      (*x).atomicOp!"+="(1);
>> }
>>
>> shared int c;
>>
>> void main() {
>>      int x;
>>      auto tid = spawn(&thread, &x); // "just" a typo
>> }
>>
>> You're saying that's ok, it should "just" compile. It 
>> shouldn't. It should produce an error and a mild electric 
>> discharge into the developer's chair.
>
> Yup. It's a typo. You passed a stack pointer to a scope that 
> outlives the caller.
> That class of issue is not on trial here. There's DIP1000, and 
> all sorts of things to try and improve safety in terms of 
> lifetimes.

I'm sorry, I'm not very good at writing "real" examples for 
things that don't exist or don't compile. End of sarcasm.

Let's come back to DIP1000 when it's actually implemented in it's 
entirety, ok? Anyway, you're nitpicking while actually missing 
the point altogether. The way `shared` is "implemented" today, 
the API (`thread` function) *requires* the caller to pass a 
`shared int*`. Implicit conversion breaks that contract.

At the highest level, the only reason for taking a `shared` 
argument is to pass that argument to another thread. That is the 
*only* way to communicate that intent via the type system for the 
time being. You're suggesting to ignore that fact. `shared` was 
supposed to protect from unshared aliasing, not silently allow it.
If you allow implicit conversion, there would literally be no way 
of knowing whether some API will access your data concurrently, 
other than plain old documentation (or sifting through it's code, 
which may not be available). This makes `shared` useless as a 
type qualifier.

> You only managed to contrive this by spawning a thread. If it 
> were just a normal function, this would be perfectly 
> legitimate, and again, that's my whole point.

I think you will agree that passing a pointer to a thread-local 
variable to another thread is not always a safe thing to do. 
Conditions do apply, which are on you (the programmer) to uphold, 
and the compiler can't help you with that. The only way the 
compiler *can* help you here is make sure you don't do that 
unintentionally. Which it won't be able to do if you allow such 
implicit conversion.