how to assign to shared obj.systime?

[Steven Schveighoffer]

On 7/13/20 3:26 AM, Arafel wrote:
> On 13/7/20 3:46, Steven Schveighoffer wrote:
>> On 7/11/20 6:15 AM, Arafel wrote:
>>>
>>> What I really miss is some way of telling the compiler "OK, I know 
>>> what I'm doing, I'm already in a critical section, and that all the 
>>> synchronization issues have been already managed by me".
>>
>> You do. It's a cast.
>>
> 
> Yes, and that's what I'm doing (although with some helper function to 
> make it look slightly less ugly), but for non-reference types I have to 
> do it every single time you use the variables, and it's annoying for 
> anything beyond trivial.
> 
> There's no way to avoid it, because at best you can get a pointer that 
> will be enough for most things, but it will show for instance if you 
> want to use it as a parameter to another function.
> 
> Also, with more complex data types like structs and AAs where not only 
> the AA itself, but also the members, keys and values become shared, it's 
> *really* annoying, because there's no easy way you can get a "fully" 
> non-shared reference, because `cast()` will *often* only remove the 
> external shared layer (I'm not sure it's always the case, it has happen 
> semi-randomly to me, and what it's worse, I don't know the rules for that).

cast() will remove as little as possible, but for most cases, including 
classes and struts, this means the entire tree referenced is now unshared.

An AA does something really useless, which I didn't realize.

If you have a shared int[int], and use cast() on it, it becomes 
shared(int)[int]. Which I don't really understand the point of.

But in any case, casting away shared is doable, even if you need to type 
a bit more.

> 
> Also, it becomes a real pain when you have to send those types to 
> generic code that is not "share-aware".
> 
> And for basic types, you'll be forced to use atomicOp all the time, or 
> again resort to pointers.

The intent is to cast away shared on the ENTIRE aggregate, and then use 
everything in the aggregate as unshared.

I can imagine something like this:

ref T unshared(T)(return ref shared(T) item) { return *(cast(T*)&item); }

with(unshared(this)) {
     // implementation using unshared things
}

I wasn't suggesting that for each time you access anything in a shared 
object, you need to do casting. In essence, it's what you are looking 
for, but just opt-in instead of automatic.

>>> Within this block, shared would implicitly convert to non-shared, and 
>>> the other way round, like this (in a more complex setup with a RWlock):
>>>
>>> ```
>>> setTime(ref SysTime t) shared {
>>>      synchronized(myRWMutex.writer) critical_section {  // From this 
>>> point I can forget about shared
>>>          time = t;
>>>      }
>>> }
>>> ```
>>
>> This isn't checkable by the compiler.
>>
> 
> That's exactly why what I propose is a way to *explicitly* tell the 
> compiler about it, like @system does for safety. I used 
> `critical_section`, but perhaps `@critical_section` would have been 
> clearer. Here is be a more explicit version specifying the variables to 
> which it applies (note that you'd be able to use "this", or leave it 
> empty and have it apply to everything):
> 
> ```
> void setTime(ref SysTime t) shared {
>      synchronized(myRWMutex.writer) {
>          @critical_section(time) {  // From this point I can forget 
> about shared
>              time = t;
>          }
>      }
> }
> ```

Yeah, this looks suspiciously like the with statement above. We seem to 
be on the same page, even if having different visions of who should 
implement it.

> Here it doesn't make a difference because the critical section is a 
> single line (so it's even longer), but if you had to use multiple 
> variables like that in a large expression, it'd become pretty much 
> impossible to understand without it:
> 
> ```
> import std;
> 
> synchronized shared class TimeCount { // It's a synchronized class, so 
> automatically locking
>      public:
>      void startClock() {
>          cast() startTime = Clock.currTime; // Here I have to cast the 
> lvalue
>          // startTime = cast(shared) Clock.currTime; // Fails because 
> opAssign is not defined for shared
>      }
>      void endClock() {
>          cast() endTime = Clock.currTime; // Again unintuitively casting 
> the lvalue
>      }
>      void calculateDuration() {
>          timeEllapsed = cast (shared) (cast() endTime - cast() 
> startTime); // Here I can also cast the rvalue, which looks more natural
>      }
> 
>      private:
>      SysTime startTime;
>      SysTime endTime;
>      Duration timeEllapsed;
> }
> ```
> 
> Non-obvious lvalue-casts all over the place, and even `timeEllapsed = 
> cast (shared) (cast() end - cast() start);`.
> 
> And that one is not even too complex... I know in this case you can 
> reorganize things, but it was just an example of what happens when you 
> have to use multiple shared variables in an expression.

You are better off separating the implementation of the shared and 
unshared parts. That is, you have synchronized methods, but once you are 
synchronized, you cast away shared and all the implementation is normal 
looking.

Compare:

class TimeCount {
     public:
     void startClock() {
         startTime = Clock.currTime;
     }
     synchronized void startClock() shared {
        (cast()this).startClock();
     }
     void endClock() {
         endTime = Clock.currTime;
     }
     synchronized void endClock() shared {
        (cast()this).endClock();
     }
     void calculateDuration() {
         timeEllapsed = endTime - startTime;
     }
     synchronized void calculateDuration() shared {
         (cast()this).calculateDuration();
     }

     private:
     SysTime startTime;
     SysTime endTime;
     Duration timeEllapsed;
}

I would imagine a mixin could accomplish a lot of this, but you have to 
be careful that the locking properly protects all the data.

A nice benefit of this approach is that no locking is needed when the 
instance is thread-local.

> Well, it's meant as a low level tool, similar to what @system does for 
> memory safety. You can't blame the compiler if you end up doing 
> something wrong with your pointer arithmetic or with your casts from and 
> to void* in your @system code, can you?

I think we may have been battling a strawman here. I assumed you were 
asking for synchronized to be this mechanism, when it seems you actually 
were asking for *any* tool. I just don't want the locking to be 
conflated with "OK now I can safely access any data because something 
was locked!". It needs to be opt-in, because you understand the risks.

I think those tools are necessary for shared to have a good story, 
whether the compiler implements it, or a library does.

-Steve