shared - i need it to be useful

[Manu]

On Wed, Oct 17, 2018 at 5:35 PM Stanislav Blinov via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
>
> On Wednesday, 17 October 2018 at 23:12:48 UTC, Manu wrote:
> > On Wed, Oct 17, 2018 at 2:15 PM Stanislav Blinov via
> > Digitalmars-d <digitalmars-d at puremagic.com> wrote:
> >>
> >> On Wednesday, 17 October 2018 at 19:25:33 UTC, Manu wrote:
> >> > On Wed, Oct 17, 2018 at 12:05 PM Stanislav Blinov via
> >> > Digitalmars-d <digitalmars-d at puremagic.com> wrote:
> >> >>
> >> >> On Wednesday, 17 October 2018 at 18:46:18 UTC, Manu wrote:
> >> >>
> >> >> > I've said this a bunch of times, there are 2 rules:
> >> >> > 1. shared inhibits read and write access to members
> >> >> > 2. `shared` methods must be threadsafe
> >> >> >
> >> >> >>From there, shared becomes interesting and useful.
> >> >>
> >> >> Oh God...
> >> >>
> >> >> void atomicInc(shared int* i) { /* ... */ }
> >> >>
> >> >> Now what? There are no "methods" for ints, only UFCS. Those
> >> >> functions can be as safe as you like, but if you allow
> >> >> implicit promotion of int* to shared int*, you *allow
> >> >> implicit races*.
> >> >
> >> > This function is effectively an intrinsic. It's unsafe by
> >> > definition.
> >>
> >> Only if implicit conversion is allowed. If it isn't, that's
> >> likely @trusted, and this:
> >>
> >> void atomicInc(ref shared int);
> >>
> >> can even be @safe.
> >
> > In this case, with respect to the context (a single int)
> > atomicInc()
> > is ALWAYS safe, even with implicit conversion. You can
> > atomicInc() a
> > thread-local int perfectly safely.
>
> Yes, *you* can. *Another* function can't unless *you* allow for
> it to be safe. You can't do that if that function silently
> assumes you gave it shared data, when in fact you did not.
>
> >> The signatures of those two functions are exactly the same.
> >> How is that different from a function taking a shared int
> >> pointer or reference?
> >
> > It's not, atomicInc() of an int is always safe with respect to
> > the int itself.
> > You can call atomicInc() on an unshared int and it's perfectly
> > fine,
> > but now you need to consider context, and that's a problem for
> > the
> > design of the higher-level scope.
> >
> > To maintain thread-safety, the int in question must be
> > appropriately contained.
>
> Exactly. And that means it can't convert to shared without my say
> so :)

It can though, because `int` doesn't have any shared methods; it's
inaccessible. You can't do anything with a shared int, so it's safe to
distribute regardless.

> > The problem is that the same as the example I presented before,
> > which I'll repeat:
> >
> > struct InvalidProgram
> > {
> >   int x;
> >   void fun() { ++x; }
> >   void gun() shared { atomicInc(&x); }
> > }
> >
> > The method gun() (and therefore the whole object) is NOT
> > threadsafe by
> > my definition, because fun() violates the threadsafety of gun().
> > The situation applies equally here that:
> > int x;
> > atomicInc(&x);
> > ++x; // <- by my definition, this 'API' (increment an int)
> > violates the threadsafety of atomicInc(), and atomicInc() is
> > therefore not threadsafe.
>
> No. The 'API' is just the atomicInc function. You, the user of
> that API, own the int. If the API wants a shared int from you,
> you have to be in agreement. You can't have any agreement if the
> API is only making promises and assumptions.

Okay, here's an idea... atomicInc() should NOT take `shared int*`,
change atomicInt() to receive int*.
We agree that `int` is not a threadsafe type, atomicInc() is not a
threadsafe method, and therefore shouldn't be shared.

If you have a shared(int)*, and you want to do un-threadsafe
atomicInc(), you need to cast to int*.
I think this is actually correct by my definition, because we
recognise int is not a threadsafe type, and atomicInc() is not a
threadsafe method, so hard-cast is required, and programmer must
appropriately validate the context when doing an unsafe operation.

> > `int` doesn't present a threadsafe API, so int is by
> > definition, NOT threadsafe. atomicInc() should be @system, and
> > not @trusted.
>
> Exactly. `int` isn't threadsafe and therefore cannot
> automatically convert to `shared int`.

It can convert to shared int, because it's inaccessible... you can't
do anything with a `shared int` except unsafe cast shared away. Once
you're in unsafe land, you are responsible for assuring correct
conditions.

> > If you intend to share an int, use Atomic!int, because it has a
> > threadsafe API.
>
> No. With current implementation of `shared`, which disallows your
> automatic promotion,
> your intent is enforced. You cannot share a local `int` unless
> *you know* it's safe to do so and therefore can cast that int to
> shared.

Yes, but we're not talking about current definition of shared. Stop
confusing the conversation.
This thread is about the rules I define.

> > atomicInc(shared int*) is effectively just an unsafe intrinsic,
> > and
>
> It is only unsafe if you allow int* to silently convert to shared
> int*. If you can't do that, you can't call `atomicInc` on an int*.

I allow conversion to shared. Assume that, and then solve.

Actually, I think I realise the problem. We're not stuck on a failure
of definition, we're stressing over an invalid API!
It's incorrect for atomicInt to receive a shared(int)*, because it
doesn't (and can't) promise thread-safety.
It must receive int*.
The implementation of Atomic which encapsulates access (guards against
unsafe access) to the int will do the proper cast.

> >> One could argue that it should be void free(ref void* p) { /*
> >> ... */ p = null; }
>
> > void *p2 = p;
> > free(p);
> > p2.crash();
>
> That's exactly analogous to what you're proposing: leaking
> `shared` references while keeping unshared data.

Right, and I'm saying it's completely acceptable and normal.
We don't need to be blocked on this matter.
It's a low-level intrinsic, it isn't a general use API. Can we please
look past it.

void atomicInc(int*); // <- we'll assume this moving forward.

As far as I can tell, this is the ONLY issue you have with the design.
Please find another issue.

> > This talk of blunt casts and "making sure everything is good"
> > is all just great, but it doesn't mean  anything interesting
> > with respect to `shared`. It should be interesting even without
> > unsafe casts.
>
> Again, I feel like you didn't see my reply. It's not talk about
> just blunt casts to make sure everything is good. You either have
> shared data to begin with, and so can share it freely, or you
> *know* that you can share this particular piece of data, even if
> it itself isn't marked `shared`, and you *assert* that by
> deliberately casting. *You* know, *you* cast, not "some function
> expects you to know, and just auto-casts".

Sorry, I can't understand this paragraph...

> >> You're missing the point, again. You have an int. You pass a
> >> pointer to it to some API that takes an int*. You continue to
> >> use your int as just an int.
>
> > You have written an invalid program. I can think of an infinite
> > number of ways to write an invalid program.
>
> No, I have not. I didn't make any promises that my data was
> shared, and I wasn't expecting it to be treated as such. I didn't
> even author that API. The other guy (the API) made wrong
> assumptions. Don't you see the difference here?

I do not. I see it black-and-white, it you write `shared` on a method,
you made a promise to do threadsafety.
You must deliver on that promise. If you do not deliver that promise,
the program is invalid.
If you *can not* deliver on that promise, then you can not attribute
the method shared (or receive a shared argument).

> As I stated previously, there's no difference between Atomic!int
> and free functions operating on shared int* (or ref shared int).
> Struct methods are sugared versions of those free functions,
> *nothing more*. That's why we have UFCS.

I'm not sure what you're saying. Atomic encapsulates guaranteed
threadsafe access to an int, whereas shared(int)* doesn't and can't.
Any threadsafe API that receives an int will necessarily receive an
Atomic!int, unless it's deliberately doing un-safety.

> > That's basic common sense. You don't pass a pointer to a
> > function if you don't know what it does with the pointer!
>
> I should know what the function does with the pointer from it's
> signature. Now, currently in D that very blurry. *Hopefully* with
> 'scope', DIP25 and DIP1000 this becomes more common. But that's
> at least what we should strive for.
> If a function takes `shared`, I better be sure I'm giving it
> `shared`.

You don't care, because you are certain that it can't do anything
that's unsafe with your object.

> >> The point is: the caller of
> >> some API *must* assert that they indeed pass shared data. It's
> >> insufficient for the API alone to "promise" taking shared data.
> >> That's the difference with promotion to `const`.
>
> > The caller doesn't care if it's true that the callee can't do
> > anything with it that's unsafe anyway.
> > We effect that state by removing all non-threadsafe access.
>
> You allow non-threadsafe access with implicit cast, not remove it.

Why do you keep saying this?
Item #1: shared instances are completely inaccessible except for
threadsafe methods!

I **ONLY** allow threadsafe access by allowing implicit conversion.

> This broken record is getting very tiresome...

It is. I don't know how to be clearer than I have been.

> Let me ask you
> this once again: *why* are you so bent on this implicit
> conversion from mutable to shared? So far the only reason I've
> seen is to just avoid writing additional methods that forward to
> `shared` methods.

Because users of an API shouldn't have to riddle their code with
manual casts to call perfectly safe functions.
That would be a plain-as-day indicator that the design is wrong.

I need a some real examples of how the implicit cast violates threadsafety...
We've identified the issue with atomicInc(), and it's the only thing
you're stuck on, and you've never picked any general holes in the
design. Please find a legitimate hole in the design...

> Most of your casts will be the other way around, will have to be
> explicit and there's nothing that can be done about that. You'll
> only have mutable->shared casts in few select cases, exactly
> because they're corner cases where you *need* to make the
> decision clear.

If shared means "threadsafe or no access", then you don't need to make
the decision clear... you don't need to make the decision at all. It's
safe by definition.
I'm trying to create a situation where users are not concerned that
they were able to use an API correctly unless they're *implementing*
threadsafety; that's when they need to think, and any requirement for
un-safety and casting should be factored into that space, not the
calling space.