On Sat, Mar 30, 2019 at 5:45 PM Andrei Alexandrescu via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 3/30/19 2:49 PM, Jonathan M Davis wrote:
> > This would also be a great opportunity to fix some of the issues with shared in druntime
>
> The problem here is, of course, that nobody knows exactly what shared is supposed to do. Not even me. Not even Walter.

As an immediate stop-gap, shared must have *no read or write access*
to data members. Simple. Make that change. Right now.
Then shared will at least be useful and not embarrassing, and you can
continue to argue about what shared means while I get to work.

> One monumental piece of work would be DIP 1021 "Define the semantics of shared". Then people who build with -dip1021 know what to do and what to expect. Now that would patch one of the bigger holes mentioned in the other post.

Sure. But in the meantime, fix the objective bug with the current semantics where you can read/write un-protected data freely. Right now. Please for the love of god.

On Sunday, 31 March 2019 at 16:18:35 UTC, Rubn wrote:

> Look at this comment as someone pointed out before:
>
> https://github.com/dlang/dmd/pull/8557#pullrequestreview-149952733

Yes, Andrei's review was a bit too strong but he did change course later in that thread.  Also recognize that the original author did not provide much motivation or justification (though, I'm guilty of that too).

I encourage contributors to please provide a short essay with your PRs to help everyone see the larger picture.  No one can pay attention to everything going on, and not everyone who reviews pull requests is intimately involved with the overall long-term objective that some PRs move forward.

It also makes things easier on the reviewer as they don't need to do so much of their own investigation to determine context (that always irritated me a little when I was reviewing; make it easy on me), and it increases the likelihood of a PR getting a review and getting accepted.

I do ask Andrei and Walter to understand that their reviews hold *much* more weight than others.  One minor objection from them makes others think "Well, Walter and Andrei don't like this, therefore my opinion is irrelevant".  The fundamental problem is there isn't any real due process to challenge and ask for reconsideration.  (e.g. https://github.com/dlang/dmd/pull/9506#issuecomment-477936480  -- No response).

Mike

When I was reviewing, it was always in the back of my mind "Are Walter and Andrei going to scold me if I merge this?", especially after they did once. In a way, we do work on behalf of Andrei and Walter and we wnat

Mike

On Monday, 1 April 2019 at 00:01:04 UTC, Mike Franklin wrote:

> When I was reviewing, it was always in the back of my mind "Are Walter and Andrei going to scold me if I merge this?", especially after they did once. In a way, we do work on behalf of Andrei and Walter and we wnat
>
> Mike

Oops, meant to delete that, but now I'm obliged to finish.

...want them to approve.

Mike

On Monday, 1 April 2019 at 00:01:04 UTC, Mike Franklin wrote:
> On Sunday, 31 March 2019 at 16:18:35 UTC, Rubn wrote:
>
>> Look at this comment as someone pointed out before:
>>
>> https://github.com/dlang/dmd/pull/8557#pullrequestreview-149952733
>
> Yes, Andrei's review was a bit too strong but he did change course later in that thread.

Not to any actionable degree, the request for change is _still_ there.

> ... The fundamental problem is there isn't any real due process to challenge and ask for reconsideration.  (e.g. https://github.com/dlang/dmd/pull/9506#issuecomment-477936480  -- No response).

Yup.

> Also recognize that the original author did not provide much
> motivation or justification (though, I'm guilty of that too).

Oh I know that, this was done specifically for Iain, and therefore I gave no justification because he knew exactly what it was for, W&A came and trashed the party. Hmm, I shouldn't be so harsh, Walter's was a reasonable request (to split the bug fix from the refactor).

Andrei's OTOH was a drive-by putdown, and he should learn a lesson from that that there exists a line which when crossed will cause people to cut their losses. In fact I think most (perhaps this is a memory retention bias, but still) of my dealings with Andrei in DMD have been drive-by, usually with bad outcomes.

On Sunday, 31 March 2019 at 22:25:30 UTC, Manu wrote:
> Sure. But in the meantime, fix the objective bug with the current semantics where you can read/write un-protected data freely. Right now. Please for the love of god.

Apropos "things to do right now", to derail the thread a little in my own interest, could I  again ask to decouple synchronized and shared for the time being? There is literally no way to write a class that has both threadsafety and invariant-safety right now, because D has no way to pull invariants into the class body without pulling in the `shared` mess. Look, if I'm using synchronized(this), as a rule I don't *care* about shared, because I'm inherently using another *kind* of thread safety to atomic primitives.

Uncoupling shared from synchronized at the class level would make synchronized classes actually usable and useful without restructuring the entire class for no gain.

On Sunday, March 31, 2019 4:25:30 PM MDT Manu via Digitalmars-d wrote:
> On Sat, Mar 30, 2019 at 5:45 PM Andrei Alexandrescu via Digitalmars-d
>
> <digitalmars-d@puremagic.com> wrote:
> > On 3/30/19 2:49 PM, Jonathan M Davis wrote:
> > > This would also be a great opportunity to fix some of the issues with shared in druntime
> >
> > The problem here is, of course, that nobody knows exactly what shared is supposed to do. Not even me. Not even Walter.
>
> As an immediate stop-gap, shared must have *no read or write access*
> to data members. Simple. Make that change. Right now.
> Then shared will at least be useful and not embarrassing, and you can
> continue to argue about what shared means while I get to work.
>
> > One monumental piece of work would be DIP 1021 "Define the semantics of shared". Then people who build with -dip1021 know what to do and what to expect. Now that would patch one of the bigger holes mentioned in the other post.
>
> Sure. But in the meantime, fix the objective bug with the current semantics where you can read/write un-protected data freely. Right now. Please for the love of god.

Honestly, that's really what shared is missing. _Some_ operations which aren't guaranteed to be thread-safe are illegal, but many aren't. With that fixed, I don't know what else shared would actually need. The stuff in core.sync really needs to be fixed up to use shared correctly (which is critical for being able to use shared cleanly), but at that point, shared itself would be doing what it needs to be doing. We could then potentially add stuff like synchronized classes on top of that, but that's just making shared easier to use, not actually required for shared to work.

- Jonathan M Davis

On Monday, April 1, 2019 1:08:29 AM MDT FeepingCreature via Digitalmars-d wrote:
> On Sunday, 31 March 2019 at 22:25:30 UTC, Manu wrote:
> > Sure. But in the meantime, fix the objective bug with the current semantics where you can read/write un-protected data freely. Right now. Please for the love of god.
>
> Apropos "things to do right now", to derail the thread a little in my own interest, could I  again ask to decouple synchronized and shared for the time being? There is literally no way to write a class that has both threadsafety and invariant-safety right now, because D has no way to pull invariants into the class body without pulling in the `shared` mess. Look, if I'm using synchronized(this), as a rule I don't *care* about shared, because I'm inherently using another *kind* of thread safety to atomic primitives.
>
> Uncoupling shared from synchronized at the class level would make synchronized classes actually usable and useful without restructuring the entire class for no gain.

How is synchronized unrelated to shared? shared is part of anything and everything in D which involves sharing data across threads. The only exception is __gshared, which is only supposed to be used for C globals (which as I understand it, can't be shared, because that wouldn't work with the linker), and you have to be _really_ careful whenever __gshared is used, because the compiler assumes that it's actually thread-local, because __gshared is not actually part of the type.

synchronized is just one way to provide a mutex for protecting shared data. The data itself still needs to be shared and then usually has to have shared cast away while the data is protected by the mutex that goes with synchronized (at which point, it's up to the programmer to ensure that no thread-local references to the data escape or exist once the mutex is unlocked). synchronized classes (which have never been fully implemented) are supposed to deal with automatically casting away the outer level of shared within the member functions, but it's still the same mechanism that you have to do now manually. If you're not using shared with synchronized, then you're doing something seriously wrong - probably using __gshared, which is just begging for trouble. In general, any object using synchronized methods should be shared.

- Jonathan M Davis

On Monday, 1 April 2019 at 10:11:04 UTC, Jonathan M Davis wrote:
> On Monday, April 1, 2019 1:08:29 AM MDT FeepingCreature via Digitalmars-d wrote:
>> Uncoupling shared from synchronized at the class level would make synchronized classes actually usable and useful without restructuring the entire class for no gain.
>
> How is synchronized unrelated to shared?

In the one way that matters - in practice. :)

I'll be honest; we use synchronized a whole lot and shared in maybe ten lines total. How? Well, in the obvious way: void method() { synchronized (this) { } }. No shared, no fuss.

I would be surprised if this was not the predominant way of synchronizing classes in D in production right now, precisely because you *don't* have to bother with shared.

And if you do it that way, invariants are inherently useless.

> The only exception is __gshared, which is only supposed to be used for C globals

I'm sorry, but there's a very great distance here, at least in our codebase, between "only supposed to" (C globals) and "what it's actually used for" (*any* threadshared globals). Nobody wants to touch shared, but we still need threading.

On Monday, 1 April 2019 at 12:28:37 UTC, FeepingCreature wrote:
> I'm sorry, but there's a very great distance here, at least in our codebase, between "only supposed to" (C globals) and "what it's actually used for" (*any* threadshared globals). Nobody wants to touch shared, but we still need threading.

Sorry for the double post, but I realized I'd forgotten to explain *why* we do it that way.

You will note that the way I explained threading is the way that basically any C-based language that doesn't have threading correctness as an explicit selling point does it: as a thin wrapper around pthreads. It's the "natural", "normal" way to do threading, the way that programmers have been doing it for literal decades. Threads and mutexes.

If D wants people to do it in a different way, that solution needs to be slick, operational and *reliable*. Any solution that starts with "well you can just cast it away cause it doesn't really work yet" doesn't have the reaction of "well we're just gonna do it that way then, because there's no other way, and hope the D devs clean up shared one of these *years*" - the reaction is going to be the entirely predictable "well, we're just gonna go back to the C way then." That's where D is right now, and that's why I look at all the discussion about shared with a kind of amused bewilderment. Thread safety is simply fundamentally not about exposing atomic access to data in our minds. Thread safety is about having classes, that own data, access to which they can lock to one thread. Synchronized(this). That's the established, commonsense, baseline idiom. It shouldn't be surprising that we prefer it to randomly casting away modifiers that the language randomly forces on us despite them being highly experimental and subject to change.

Get the normal, established way to do threading working consistently and reliably *now*. Then think about shared.

That's my opinion, at least.

On 19.03.19 03:52, Andrei Alexandrescu wrote:
> 
> Turns out the second clause fails. That takes us to the definition of empty in the same module:
> 
> @property bool empty(T)(auto ref scope const(T) a)
> if (is(typeof(a.length) : size_t))
> {
>      return !a.length;
> }
> ...

Why should length be required to be `const` when almost everything that's `const` is not even a range?

> The intent is fairly clear - if a range defines empty as a size_t (somewhat oddly relaxed to "convertible to size_t"), then empty can be nicely defined in terms of length. Cool. But empty doesn't work with TestAliasedString due to an overlooked matter: the "const". A mutable TestAliasedString converts to a string, but a const or immutable TestAliasedString does NOT convert to a const string! So this fixes that matter:
> 
>      struct TestAliasedString
>      {
>          string get() @safe @nogc pure nothrow { return _s; }
>          const(string) get() @safe @nogc pure nothrow const { return _s; }
>          alias get this;
>          @disable this(this);
>          string _s;
>      }
> 

Reasoning from first principles, the right fix is actually to allow non-const length.

> That makes empty() work, but also raises a nagging question: what was the relationship of TestAliasedString to string before this change? Surely that wasn't subtyping. (My response would be: "Odd.") And why was Phobos under the obligation to cater for such a type and its tenuous relationship to a range?

The problem isn't `alias this`. It's `const`. The following type does not pass isInputRange either:

struct ApparentlyNotARange{
    size_t k=10;
    int front(){ return 0; }
    void popFront(){ --k; }
    @property size_t length(){ return k; }
}

While this does:

struct ApparentlyNotARange{
    size_t k=10;
    int front(){ return 0; }
    void popFront(){ --k; }
    @property size_t length()const{ return k; }
}

The reason why Phobos ought to be under the obligation to cater for such a type is because failure to do so is an artificial limitation. It works by default, but someone went out of their way to make sure it does not work if `length` is not annotated `const` (or `@property`). This is particularly strange because there is no such thing as a const range.