August 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Jonathan M Davis | 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<jdrewsen@nospam.com> wrote: >>> Den 17-08-2011 15:51, Steven Schveighoffer skrev: >>>> On Wed, 17 Aug 2011 05:43:00 -0400, Jonas Drewsen >>>> <jdrewsen@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<jdrewsen@nospam.com> >>>>>>> >>>>>>> wrote: >>>>>>>> Den 16-08-2011 18:55, Martin Nowak skrev: >>>>>>>>> On Tue, 16 Aug 2011 15:13:40 +0200, >>>>>>>>> dsimcha<dsimcha@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) > In the > other case, the type system guarantees that the data is thread-local and > therefore thread-safe, and you're breaking that guarantee by casting it to > shared. On the other end, you're then casting it back, since you know that the > data isn't actually shared. In both cases, you're circumventing the compiler's > guarantees. In both cases, you've claimed that it's thread for th second > thread to use the data, when if you screwed up and left references to it in > the first thread, then it isn't. I don't really see the difference. > > - Jonathan M Davis You don't break any guarantees when casting away shared if you know that the data is actually not shared anymore. (Of course, if it is actually still shared between multiple threads, this is about as bad as altering data which is typed as immutable somewhere.) You don't break any guarantees if you don't actually break them. The casts are just there because the compiler is unable to verify that you don't. Therefore casting to immutable and back and then changing data is bad, but casting data to shared, transferring ownership to another single thread and then casting back to unshared is good. | |||
Permalink
ReplyAugust 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Jonathan M Davis | On 2011-08-17 09:13, Jonathan M Davis wrote: > On Wednesday, August 17, 2011 09:01:02 Jacob Carlborg wrote: >> On 2011-08-16 22:01, Jonathan M Davis wrote: >>> On Tuesday, August 16, 2011 18:47:44 Jacob Carlborg wrote: >>>> BTW, do we have style guides for abbreviations like this? >>> >>> No. The style guide says camelcase, but it doesn't specify how to deal >>> with abbreviations. Personally, I think that it should be dealt with on >>> a case by case basis, since what works best depends on the >>> abbreviation, but in general, I prefer all caps. >>> >>> - Jonathan M Davis >> >> There's also the question when having abbreviations in a method name: >> >> sendToHTTP >> >> or >> >> sendToHttp >> >> In general think abbreviations should be avoided except when the >> abbreviation is better known, e.g. HTTP, FTP and so on. I like camlecase >> better than all caps both for class names and method names. > > I meant that I generally prefer all caps for the abbreviations (e.g. > sentToHTTP rather than sendToHttp). I definitely think that the function names > should be camelcased and the types should be pascal cased - which is what the > d style is. And yes, abbreviations should only be used when they're > appropriately clear. Function names should be as short as we can reasonably > make them and still have them be clear, but clarity ranks higher than brevity > when it comes to picking good function names IMHO. > > - Jonathan M Davis Ok, seems we agree. -- /Jacob Carlborg | |||
August 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Alix Pexton | Den 17-08-2011 00:17, Alix Pexton skrev:
> On 16/08/2011 20:55, jdrewsen wrote:
>> Den 16-08-2011 16:43, Alix Pexton skrev:
>>> On 16/08/2011 12:48, 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
>>>
>>> Spotted another issue...
>>>
>>> In the example for Protocol.onReceive, there is a mismatch between the
>>> parameter names specified for the delegate and those used inside it.
>>
>> you mean this one:
>>
>> client.onReceive = (ubyte[] data) { writeln("Got data", cast(char[])
>> data); return data.length;};
>>
>> I cannot identify the problem?
>
> My mistake, I was in a hurry and mixed up the functions ><
> This is the snippet in question (lines 414-417 in the source)
>
>> http.onProgress = delegate int(double dl, double dln, double ul,
>> double ult) {
>> writeln("Progress: downloaded ", dln, " of ", dl);
>> writeln("Progress: uploaded ", uln, " of ", ul);
>> };
>
> The last parameter is named "ult", but the writeln is passed "uln", I
> think more descriptive names might help.
>
> A...
ok
Thanks
Jonas
| |||
August 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Timon Gehr | On Wed, 17 Aug 2011 18:21:00 +0200, Timon Gehr <timon.gehr@gmx.ch> wrote: > 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<jdrewsen@nospam.com> wrote: >>>> Den 17-08-2011 15:51, Steven Schveighoffer skrev: >>>>> On Wed, 17 Aug 2011 05:43:00 -0400, Jonas Drewsen >>>>> <jdrewsen@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<jdrewsen@nospam.com> >>>>>>>> >>>>>>>> wrote: >>>>>>>>> Den 16-08-2011 18:55, Martin Nowak skrev: >>>>>>>>>> On Tue, 16 Aug 2011 15:13:40 +0200, >>>>>>>>>> dsimcha<dsimcha@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) > >> In the >> other case, the type system guarantees that the data is thread-local and >> therefore thread-safe, and you're breaking that guarantee by casting it to >> shared. On the other end, you're then casting it back, since you know that the >> data isn't actually shared. In both cases, you're circumventing the compiler's >> guarantees. In both cases, you've claimed that it's thread for th second >> thread to use the data, when if you screwed up and left references to it in >> the first thread, then it isn't. I don't really see the difference. >> >> - Jonathan M Davis > > You don't break any guarantees when casting away shared if you know that the data is actually not shared anymore. > (Of course, if it is actually still shared between multiple threads, this is about as bad as altering data which is typed as immutable somewhere.) > > > You don't break any guarantees if you don't actually break them. The casts are just there because the compiler is unable to verify that you don't. > > Therefore casting to immutable and back and then changing data is bad, but casting data to shared, transferring ownership to another single thread and then casting back to unshared is good. Not wanting to drift too far off topic I'll add one last point. given: immutable(int[]) data = assumeUnique(myints); send(cast(int[])data); writeln(data[0]); A compiler implementation could deduce data won't change between initialization and the write. Thus performing optimizations that would break the code. I think that and being able to store ctfe data in read only sections are the reasons for the undefined behavior. Removing Immutable With A Cast => http://www.digitalmars.com/d/2.0/const3.html martin | |||
August 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 | On Wednesday, August 17, 2011 13:31 Martin Nowak wrote:
> Not wanting to drift too far off topic I'll add one last point.
> given:
> immutable(int[]) data = assumeUnique(myints);
> send(cast(int[])data);
> writeln(data[0]);
>
> A compiler implementation could deduce data won't change between
> initialization and the write.
> Thus performing optimizations that would break the code.
> I think that and being able to store ctfe data in read only sections are
> the reasons for the undefined behavior.
> Removing Immutable With A Cast =>
> http://www.digitalmars.com/d/2.0/const3.html
Yes, because you actually kept the data on the original thread. The _only_ way that passing via send by casting to and from share or to and from immutable is going to work properly if you _do not keep the data on the original thread_. By casting to either shared or immutable, you're breaking the guarantees that the compiler makes and expects. So, doing something like using that data again on the original thread is broken regardless. The _only_ time that this way of passing mutable data via send is okay is when you're actually passing full ownership of the data across and never touching it on the original thread again (unless you pass it back across the same way, handing over ownership again). Your code is broken regardless of whether you're using immutable or shared. True, immutable might be more thoroughly optimized than shared is therefore more likely to break, but the code is broken regardless.
- Jonathan M Davis
| |||
August 17, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | On 08/17/2011 11:09 PM, Jonathan M Davis wrote:
> On Wednesday, August 17, 2011 13:31 Martin Nowak wrote:
>> Not wanting to drift too far off topic I'll add one last point.
>> given:
>> immutable(int[]) data = assumeUnique(myints);
>> send(cast(int[])data);
>> writeln(data[0]);
>>
>> A compiler implementation could deduce data won't change between
>> initialization and the write.
>> Thus performing optimizations that would break the code.
>> I think that and being able to store ctfe data in read only sections are
>> the reasons for the undefined behavior.
>> Removing Immutable With A Cast =>
>> http://www.digitalmars.com/d/2.0/const3.html
>
> Yes, because you actually kept the data on the original thread. The _only_ way
> that passing via send by casting to and from share or to and from immutable is
> going to work properly if you _do not keep the data on the original thread_.
> By casting to either shared or immutable, you're breaking the guarantees that
> the compiler makes and expects. So, doing something like using that data again
> on the original thread is broken regardless. The _only_ time that this way of
> passing mutable data via send is okay is when you're actually passing full
> ownership of the data across and never touching it on the original thread
> again (unless you pass it back across the same way, handing over ownership
> again). Your code is broken regardless of whether you're using immutable or
> shared. True, immutable might be more thoroughly optimized than shared is
> therefore more likely to break, but the code is broken regardless.
>
> - Jonathan M Davis
No, casting shared and back carefully does not break the code, as I pointed out in my previous post. Casting immutable and back does.
You don't break the guarantees the compiler expects and makes with transferring ownership by cast to shared and back.
int // this variable is visible to one thread only
shared(int) // this variable might be visible to multiple threads
If you *know* that your shared variable is visible by only one thread, you can safely cast it to unshared, without breaking any guarantees.
You can always safely cast to shared, because shared gives less guarantees.
Doing it by casting to immutable and back, on the other hand, is wrong.
int // this variable may change its value
immutable(int) // this variable may not
If you *know* that your mutable variable is never going to change, you can safely cast it to immutable, without breaking any guarantees.
If you *know* that your immutable variable is never going to change after the cast, you can safely cast it to mutable, without breaking any guarantees.
But in this case it is probably going to change, so it does not work with immutable. shared and immutable are two very different things.
| |||
August 18, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Jonas Drewsen | 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 One more comment: In various places, you have someFunction(in const(char)[] someString). Const and in are redundant. In is synonymous with const. Also, anyone have any luck getting Curl libs to work with DMD on Windows? The binaries available at http://curl.haxx.se/download.html look like they're COFF. (<rant> Why do we still use freakin' OMF? </rant>) Also, the source doesn't look like it's pleasant to compile. | |||
August 18, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to dsimcha Attachments:
| On Wed, Aug 17, 2011 at 10:41 PM, dsimcha <dsimcha@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<https://github.com/jcd/phobos/blob/curl-wrapper/etc/curl.d> Docs: http://freeze.steamwinter.com/**D/web/phobos/etc_curl.html<http://freeze.steamwinter.com/D/web/phobos/etc_curl.html>
>>
>> Demolish!
>>
>> /Jonas
>>
>
> One more comment: In various places, you have someFunction(in
> const(char)[] someString). Const and in are redundant. In is synonymous
> with const.
>
> Also, anyone have any luck getting Curl libs to work with DMD on Windows?
> The binaries available at http://curl.haxx.se/download.**html<http://curl.haxx.se/download.html>look like they're COFF. (<rant> Why do we still use freakin' OMF? </rant>)
> Also, the source doesn't look like it's pleasant to compile.
>
I've always been using etc.c.curl on Windows. Use implib to generate .lib files from the dlls.
| |||
August 18, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
 Posted in reply to Jonas Drewsen | Jonas Drewsen: >If a decent package management tool was added to D which could handle downloading the binary lib for you then wouldn't that be an okey solition for you?< What I think is acceptable is to put now the binary of the Windows library inside the zip of the DMD distribution. Once that decent package management tool will be present and working on Windows, you will be free to remove the binary lib, if you want (but I suggest to not do it, because D std library is suppsed to have few batteries included, and today accessing the web is a basic need. Maybe even the package management tool needs a curl to download the files, so we are back to the start). ---------------- dsimcha: >Also, anyone have any luck getting Curl libs to work with DMD on Windows? The binaries available at http://curl.haxx.se/download.html look like they're COFF. (<rant> Why do we still use freakin' OMF? </rant>) Also, the source doesn't look like it's pleasant to compile.< See my answer above :-) If I want to use DMD to write a 20 lines long script-line program I will not compile a C lib (and probably a 200 lines long program too). Bye, bearophile | |||
August 18, 2011 Re: CURL review request | ||||
|---|---|---|---|---|
| ||||
Posted in reply to bearophile | On Thursday, August 18, 2011 04:37:20 bearophile wrote:
> Jonas Drewsen:
> >If a decent package management tool was added to D which could handle downloading the binary lib for you then wouldn't that be an okey solition for you?<
> What I think is acceptable is to put now the binary of the Windows library inside the zip of the DMD distribution. Once that decent package management tool will be present and working on Windows, you will be free to remove the binary lib, if you want (but I suggest to not do it, because D std library is suppsed to have few batteries included, and today accessing the web is a basic need. Maybe even the package management tool needs a curl to download the files, so we are back to the start).
>
> ----------------
>
> dsimcha:
> >Also, anyone have any luck getting Curl libs to work with DMD on Windows? The binaries available at http://curl.haxx.se/download.html look like they're COFF. (<rant> Why do we still use freakin' OMF? </rant>) Also, the source doesn't look like it's pleasant to compile.<
> See my answer above :-) If I want to use DMD to write a 20 lines long script-line program I will not compile a C lib (and probably a 200 lines long program too).
It would be a good idea to provide links to to such a binary (probably in the documentation), but I definitely don't think that any such thing should be provided with dmd itself. It increases the size of the zip file unnecessarily for the majority of users, and anyone using anything other than Windows definitely won't need it.
So yes, we need to make it easy for people to find the appropriate binaries for Windows, but no, we shouldn't distribute them with dmd.
- Jonathan M Davis
| |||
Copyright © 1999-2021 by the D Language Foundation