Possible change to array runtime?

[Steven Schveighoffer]

On Thu, 13 Mar 2014 13:44:01 -0400, monarch_dodra <monarchdodra at gmail.com>  
wrote:

> On Thursday, 13 March 2014 at 16:17:17 UTC, Steven Schveighoffer wrote:
>> On Thu, 13 Mar 2014 11:53:15 -0400, monarch_dodra  
>> <monarchdodra at gmail.com> wrote:
>>> Please keep in mind that if the objects stored are RAII, then if/when  
>>> we will have a finalizing GC, the stomped elements will have been  
>>> leaked.
>>>
>>> Clobbering elements is more than just "I won't use these elements  
>>> anymore", it's "I won't use them, and they are safe to be discarded of  
>>> right now".
>>>
>>> In know that's a big "if", but it could happen. If we go the way of  
>>> your proposal, we are definitively closing that door.
>>
>> I'm not understanding this. Can you explain further/give example?
>
> Well, image "File": It's a struct that owns a "file handle", and when  
> the struct is destroyed, it releases/closes the file. Basic RAII.
>
> Now, imagine we want to open several files. We'd use:
> File[] files;
>
> "As of today", this does not end well, as the GC does not finalize the  
> array elements, and the file handles are leaked. We could hope, that one  
> day, the GC *will* finalize the array elements.
>
> However, with your proposal, imagine this code:
>
> File[] files;
> files ~= File("foo"); //Opens file "foo"
> files.length = 0;
> files ~= File("bar"); //Clobbers "foo"
>
> With this setup, the File handling "foo" gets clobbered, ruining any  
> chance of releasing it ever.

Line 3 should be files = null. There is no point to setting length to 0,  
and mostly this is a relic from D1 code. That was my basis of why it  
shouldn't cause problems.

> The "only way" to make it work (AFAIK), would be for "length = 0" to  
> first finalize the elements in the array. However, you do that, you may  
> accidentally destroy elements that are still "live" and referenced by  
> another array.

In fact, assumeSafeAppend *should* finalize the elements in the array, if  
it had that capability. When you call assumeSafeAppend, you are telling  
the runtime that you are done with the extra elements.

> I'm not too hot about this proposal. My main gripe is that while "length  
> = 0" *may* mean "*I* want to discard this data", there is no guarantee  
> you don't have someone else that has a handle on said data, and sure as  
> hell doesn't want it clobbered.

Again, the =null is a better solution. There is no point to keeping the  
same block in reference, but without any access to elements, unless you  
want to overwrite it.

The problem is that we have this new mechanism that keeps those intact. If  
I could have imagined this outcome and was aware of this logic, I would  
have kept the length = 0 mechanics from D1 to begin with.

> For what it's worth, I think the problem would go away all by itself if  
> "assumeSafeAppend" had more exposition, and was actually used. I think a  
> simpler solution would be to simply educate users of this function, and  
> promote its use. Its simpler than adding a special case language change.

The issue I'm examining is that people are reluctant to move off of D1,  
because their D1 code behaves well when they do length = 0, and it behaves  
badly in D2, even though it compiles and works correctly. They do not have  
assumeSafeAppend in D1. I'm unsure why they have not used it, either out  
of ignorance of the function or they have decided it's not worth the  
effort, I have no idea.

>> https://github.com/D-Programming-Language/druntime/pull/147
>>
>> reserve and capacity were made nothrow, not sure why assumeSafeAppend  
>> shouldn't also be.
>>
>
> The irony is that reserve can't actually be tagged pure nor nothrow:  
> Since it can cause relocation, it can call postblit, which in turn may  
> actually be impure or throw.
>
> assumeSafeAppend, on the other hand, is *certifiably* pure and nothrow,  
> since it never actually touches any data.

It does touch data, but it should be pure and nothrow.

> I had opened a pull to fix this, but it was never merged, due to the  
> back and forth "fiasco" of tagging said reserve. Since I'm not fluent in  
> D-runtime, I just let the issue drop.

Which PR?

-Steve