On Friday, August 19, 2011 04:58 Timon Gehr wrote: > On 08/19/2011 01:50 AM, Jonathan M Davis wrote: > > On Thursday, August 18, 2011 16:00 Timon Gehr wrote: > >> On 08/19/2011 12:35 AM, Jonathan M Davis wrote: > >>> On Thursday, August 18, 2011 14:33 jdrewsen wrote: > >>>> Den 17-08-2011 18:21, Timon Gehr skrev: > >>>>> On 08/17/2011 05:58 PM, Jonathan M Davis wrote: > >>>>>> On Wednesday, August 17, 2011 11:30:06 Steven Schveighoffer wrote: > >>>>>>> On Wed, 17 Aug 2011 11:05:56 -0400, jdrewsen<jdrew...@nospam.com> > > > > wrote: > >>>>>>>> Den 17-08-2011 15:51, Steven Schveighoffer skrev: > >>>>>>>>> On Wed, 17 Aug 2011 05:43:00 -0400, Jonas Drewsen > >>>>>>>>> <jdrew...@nospam.com> > >>>>>>>>> > >>>>>>>>> wrote: > >>>>>>>>>> On 17/08/11 00.21, Jonathan M Davis wrote: > >>>>>>>>>>> On Tuesday, August 16, 2011 12:32 Martin Nowak wrote: > >>>>>>>>>>>> On Tue, 16 Aug 2011 20:48:51 +0200, > >>>>>>>>>>>> jdrewsen<jdrew...@nospam.com> > >>>>>>>>>>>> > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>> Den 16-08-2011 18:55, Martin Nowak skrev: > >>>>>>>>>>>>>> On Tue, 16 Aug 2011 15:13:40 +0200, > >>>>>>>>>>>>>> dsimcha<dsim...@yahoo.com> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> On 8/16/2011 7:48 AM, Jonas Drewsen wrote: > >>>>>>>>>>>>>>>> Hi all, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> This is a review request for the curl wrapper. Please > >>>>>>>>>>>>>>>> read the > >>>>>>>>>>>>>>>> "known > >>>>>>>>>>>>>>>> issues" in the top of the source file and if possible > >>>>>>>>>>>>>>>> suggest a > >>>>>>>>>>>>>>>> solution. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> We also need somebody for running the review process. > >>>>>>>>>>>>>>>> Anyone? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Code: > >>>>>>>>>>>>>>>> https://github.com/jcd/phobos/blob/curl-wrapper/etc/curl > >>>>>>>>>>>>>>>> .d > >>>>>>>>>>>>>>>> Docs: > >>>>>>>>>>>>>>>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Demolish! > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> /Jonas > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> From a quick look, this looks very well thought out. I'll > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> review > >>>>>>>>>>>>>>> it > >>>>>>>>>>>>>>> more thoroughly when I have more time. A few > >>>>>>>>>>>>>>> questions/comments > >>>>>>>>>>>>>>> from a > >>>>>>>>>>>>>>> quick look at the docs: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Does the async stuff use D threads, or does Curl have its > >>>>>>>>>>>>>>> own > >>>>>>>>>>>>>>> async > >>>>>>>>>>>>>>> API? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> In your examples for postData, you have onReceive a > >>>>>>>>>>>>>>> ubyte[] and > >>>>>>>>>>>>>>> write > >>>>>>>>>>>>>>> it out to console. Did you mean to cast this to some kind > >>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>> string? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> For onReceive, what's the purpose of the return value? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> If/when this module makes it into Phobos, are we going to > >>>>>>>>>>>>>>> start > >>>>>>>>>>>>>>> including a libcurl binary with DMD distributions so that > >>>>>>>>>>>>>>> std.curl > >>>>>>>>>>>>>>> feels truly **standard** and requires zero extra > >>>>>>>>>>>>>>> configuration? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> I was also wondering about the async handling. In the > >>>>>>>>>>>>>> long-term > >>>>>>>>>>>>>> I'd like > >>>>>>>>>>>>>> to see a bigger picture for async handling in phobos > >>>>>>>>>>>>>> (offering > >>>>>>>>>>>>>> some kind > >>>>>>>>>>>>>> of futures, maybe event-loops etc.). > >>>>>>>>>>>>>> Though this is not a requirement for the curl wrapper now. > >>>>>>>>>>>>>> std.parallelism also has some kind of this stuff and file > >>>>>>>>>>>>>> reading > >>>>>>>>>>>>>> would > >>>>>>>>>>>>>> benefit from it too. > >>>>>>>>>>>>> > >>>>>>>>>>>>> This has been discussed before and I also think this is very > >>>>>>>>>>>>> important. > >>>>>>>>>>>>> But before that I think some kind of package management > >>>>>>>>>>>>> should be > >>>>>>>>>>>>> prioritized (A DIP11 implementaion or a more traditional > >>>>>>>>>>>>> solution). > >>>>>>>>>>>>> > >>>>>>>>>>>>>> One thing I spotted at a quick glance, sending to be filled > >>>>>>>>>>>>>> buffers to > >>>>>>>>>>>>>> another thread should not be done by casting to shared not > >>>>>>>>>>>>>> immutable. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I'm not entirely sure what you mean. There is no use of > >>>>>>>>>>>>> shared buffers > >>>>>>>>>>>>> in the wrapper. I do cast the buffer between > >>>>>>>>>>>>> mutable/immutable because > >>>>>>>>>>>>> only immutable or by value data can be passed using > >>>>>>>>>>>>> std.concurrency. > >>>>>>>>>>>>> Since the buffers are only used by the thread that currently > >>>>>>>>>>>>> has the > >>>>>>>>>>>>> buffer this is safe. I've previously asked for a non-cast > >>>>>>>>>>>>> solution > >>>>>>>>>>>>> (ie. > >>>>>>>>>>>>> some kind of move between threads semantic for > >>>>>>>>>>>>> std.concurrency) but > >>>>>>>>>>>>> was > >>>>>>>>>>>>> advised that this was the way to do it. > >>>>>>>>>>>>> > >>>>>>>>>>>>>> martin > >>>>>>>>>>>> > >>>>>>>>>>>> Pardon the typo. What I meant is that AFAIK casting from > >>>>>>>>>>>> immutable to > >>>>>>>>>>>> mutable has undefined behavior. > >>>>>>>>>>>> The intended method for sending a uint[] buffer to another > >>>>>>>>>>>> thread is > >>>>>>>>>>>> to > >>>>>>>>>>>> cast that > >>>>>>>>>>>> buffer to shared (cast(shared(uint[])) and casting away the > >>>>>>>>>>>> shared > >>>>>>>>>>>> on the > >>>>>>>>>>>> receiving side. > >>>>>>>>>>>> It is allowed to send shared data using std.concurrency. > >>>>>>>>>>> > >>>>>>>>>>> Casting away immutability and then altering data is undefined. > >>>>>>>>>>> Actually > >>>>>>>>>>> casting it away is defined. So, if you have data in one thread > >>>>>>>>>>> that > >>>>>>>>>>> you know > >>>>>>>>>>> is unique, you can cast it to immutable (or > >>>>>>>>>>> std.exception.assumeUnique to do > >>>>>>>>>>> it) and then send it to another thread. On that thread, you can > >>>>>>>>>>> then > >>>>>>>>>>> cast it > >>>>>>>>>>> to mutable and alter it. > >>>>>>>>>>> > >>>>>>>>>>> However, you're circumventing the type system when you do this. > >>>>>>>>>>> So, > >>>>>>>>>>> you have > >>>>>>>>>>> to be very careful. You're throwing away the guarantees that > >>>>>>>>>>> the compiler > >>>>>>>>>>> makes with regards to const and immutable. It _is_ guaranteed > >>>>>>>>>>> to work > >>>>>>>>>>> though. > >>>>>>>>>>> And I'm not sure that there's really any difference between > >>>>>>>>>>> casting > >>>>>>>>>>> to shared > >>>>>>>>>>> and back and casting to immutable and back. In both cases, > >>>>>>>>>>> you're circumventing the type system. The main difference > >>>>>>>>>>> would be that if > >>>>>>>>>>> you > >>>>>>>>>>> screwed up with immutable and cast away immutable on something > >>>>>>>>>>> that > >>>>>>>>>>> really was > >>>>>>>>>>> immutable rather than something that you cast to immutable just > >>>>>>>>>>> to send it to > >>>>>>>>>>> another thread, then you could a segfault when you tried to > >>>>>>>>>>> alter it, > >>>>>>>>>>> since it > >>>>>>>>>>> could be in ROM. > >>>>>>>>>>> > >>>>>>>>>>> - Jonathan M Davis > >>>>>>>>>> > >>>>>>>>>> Yeah I know you have to be careful when doing these kind of > >>>>>>>>>> things. I > >>>>>>>>>> ran into the problem of sending buffers between threads (using > >>>>>>>>>> std.concurrency) so that they could be reused. There isn't any > >>>>>>>>>> "move ownership" support in place so Andrei suggested I could do > >>>>>>>>>> it by casting immutable. > >>>>>>>>>> > >>>>>>>>>> If anyone knows of a cleaner way to do this please tell. > >>>>>>>>> > >>>>>>>>> casting to shared and back. Passing shared data should be > >>>>>>>>> supported by std.concurrency, and casting away shared is defined > >>>>>>>>> as long as you know > >>>>>>>>> only one thread owns the data after casting. > >>>>>>>>> > >>>>>>>>> -Steve > >>>>>>>> > >>>>>>>> Why is this cleaner than casting to immutable and back? > >>>>>>> > >>>>>>> Once it's immutable, it can never be mutable again. Casting to > >>>>>>> immutable is a one-way street. Yes, you can cast to mutable, but > >>>>>>> you still can't change the data unless you want undefined > >>>>>>> behavior. > >>>>>>> > >>>>>>> Shared is not like that, an item can be thread-local, then shared, > >>>>>>> then thread local again, all the time being mutable. It also > >>>>>>> reflects better what the process is (I'm sharing this data with > >>>>>>> another thread, then that > >>>>>>> thread is taking ownership). There's still the possibility to screw > >>>>>>> up, but at least you are not in undefined territory in the > >>>>>>> correctly-implemented case. > >>>>>> > >>>>>> Are you sure? As I understand it, there's no real difference between > >>>>>> casting to > >>>>>> immutable and back and casting to shared and back. Both circumvent > >>>>>> the type > >>>>>> system. In the one case, the type system guarantees that the data > >>>>>> can't be > >>>>>> altered, and you're breaking that guarantee, because you know that > >>>>>> it _can_ > >>>>>> be, since you created the data and know that it's actually mutable. > >>>>> > >>>>> No. As soon as the data is typed as immutable anywhere it cannot be > >>>>> changed anymore. You only break guarantees if you actually try to > >>>>> change the data (otherwise std.typecons.assumeUnique would perform > >>>>> its job outside defined behavior btw) > >>>> > >>>> I'm thinking down the same lines as Jonathan. Is the behavior for > >>>> immutable casts that you describe specified in the language reference > >>>> somewhere? > >>>> > >>>> I have no problem with using shared casts instead of immutable - I > >>>> just want make sure it is really needed. > >>> > >>> The behavior of casting a way const or immutable on a value and then > >>> mutating it is undefined by the language, because you're breaking the > >>> language's guarantees and what happens depends entirely on whether the > >>> actual object was actually immutable. However, in the case of casting > >>> to immutable and then casting back, you _know_ that the object is > >>> mutable, so there's no problem. You're just circumventing the type > >>> system which throws away the guarantees that it gives you about > >>> immutability, which could screw up optimizations if you had actually > >>> did more than just pass the variable around. But that's just not > >>> happening here. > >>> > >>> As for casting to and from shared and mutating the object, I don't see > >>> how it is any more defined than casting to and from immutable and then > >>> mutating the object is. In both cases, you circumvented the type > >>> system, which breaks the compiler's guarantees and risks bugs if you > >>> actually do more than just pass the variable around before casting it > >>> back to being thread-local and mutable. > >>> > >>> - Jonathan M Davis > >> > >> As long as the data is not being shared between multiple threads after > >> it's sharedness has been cast away, you are well in defined area, > >> because you are NOT breaking anything. > > > > The _only_ reason that you're not breaking anything is because you are > > being careful and making sure that the data is not actually shared > > between threads. You're _still_ circumventing the type system and > > breaking the guarantee that a non-shared variable is not shared between > > threads. _You_ are the one guaranteeing that the variable is only on one > > thread, not the compiler. And when you cast away immutable, _you_ are > > the one guaranteeing that immutable data is not being mutated, not the > > compiler. And in this case, you can make that guarantee in exactly the > > same way that you can guarantee that the variable which was cast to > > shared and back to be passed across threads isn't actually shared > > between threads once it has been passed. > > > >> The crucial difference between immutable and shared is, that something > >> that is immutable will always be immutable, but being shared or not may > >> change dynamically. > >> > >> Casting to immutable is a one-way-street, while casting to shared is > >> not. > > > > Casting to immutable does not make the data magically immutable. > > Yes it does. That is quite exactly the definition of it. > > > It makes it > > so that the compiler treats it as immutable. > > The compiler is irrelevant, the fact that one compiler generates > assembly that behaves as you'd like it to behave does not mean the code > is valid. > > > Casting from immutable makes it > > so that the compiler treats it as mutable. It does not alter whether the > > data is actually immutable. > > 'Actually immutable' means that the variable is never changed. The > language spec says that if an immutable variable is not 'actually > immutable', your program is in error, even if immutability has been > explicitly cast away. > > > Casting away immutable and altering data is undefined, > > because it depends entirely on whether the data is actually immutable or > > not. > > Just to make sure we agree on that: knowing why it is undefined does not > imply there are cases where it is actually defined. > > > If it isn't, and you don't have any other references to the data (or none > > of the other references are immutable), then you don't break _anything_ > > by mutating the variable after having cast away immutable. > > If you believe the spec, you do break validity of your code even if the > variable is not visible from another place after the cast. There is > currently afaik no reason why such code would *have* to be broken, but > that is what the spec says, because it categorically bans changing > variables that were cast from immutable. > > > With both shared and immutable, casting it away is circumnventing the > > type system. In both cases, _you_ must make sure that you code in a way > > which does not violate the guarantees that the compiler normally makes > > about those types. If you do violate those guarantees (e.g. by sharing > > non-shared data across threads or by mutating data which really was > > immutable), then you have a bug, and what happens is dependent on the > > compiler implementation and on your code. > > Agreed, and this means what happens is undefined and such code is invalid. > > > But if you don't violate those guarantees, then your program is fine. > > It's the same for both shared and immutable. It's just that the bugs > > that you're likely to get when you violate those guarantees are likely > > to be quite different. > > > > - Jonathan M Davis > > Yes, I agree. And in this specific case you do violate the language > guarantees about immutable variables when you change the variable after > casting it back to mutable. Even if it was typed mutable sometime > before, and even if it is the sole reference in the whole program. > Therefore the behavior of the current implementation is undefined under > the current spec, and changing the transfer protocol to use shared > instead of immutable would fix this. (while generating near-identical > assembly output with the current DMD)
What I completely disagree with you on is that casting something to shared, passing it to another thread, casting from shared, and then altering a variable is any more defined than casting to and from immutable and altering a variable is. In _both_ cases, you are circumventing the compiler, and in _both_ cases, _you_ must guarantee that what you're doing doesn't actually violate what the compiler is trying to guarantee. - Jonathan M Davis
