Memory leak in foreach

Andrea Fontana nospam at example.org
Wed Feb 20 14:22:51 UTC 2019


On Wednesday, 20 February 2019 at 10:50:50 UTC, aliak wrote:
> On Wednesday, 20 February 2019 at 09:31:30 UTC, Andrea Fontana 
> wrote:
>> This subtle bug[1] and its memory leak are still here because 
>> my pull request[2] had no success.
>>
>> It worths noting that since each() in std.algorithm uses a 
>> foreach+ref to iterate  structs[3], this memory leak probably 
>> is silently affecting not only me :)
>>
>> I hope someone could fix this.
>>
>> [1] https://issues.dlang.org/show_bug.cgi?id=11934
>> [2] https://github.com/dlang/dmd/pull/8437
>> [3] 
>> https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L983
>
> Does that patch ignore the ref when you specify it in a foreach 
> loop?
>
> If it does, then that sounds like it should be a compiler error 
> and not ignored. A compiler should not change the meaning of my 
> code silently just to make compilation work. We have javascript 
> for that no?
>
> Or did I misunderstand the patch?

Let me explain better.

Currently this is what is happening (if I'm right!):
- You have a range with struct elements
- You try to iterate this range using a foreach+ref.
- This range returns a copy of each element (a temporary value)
- You work on this copy by ref (it doesn't even make sense to use 
a ref to a  temporary value)
- Compiler doens't free this copy because it thinks it's a 
reference! <- leak

My idea: fix the current behaviour, freeing the elements created 
and then fix the language with a 
warning/error/deprecation/DIP/whatever

Please consider that throwing an error is going to brake all code 
using (for example) each() to iterate a range returning a struct, 
probably.

Should it throw an error? Don't know. Probably yes. But I feel it 
could add a lot of complexity to code: if you're writing generic 
code you have always to check if you're iterating a ref-returning 
range or not. In first case you can use foreach(ref) in the 
second case you have to use foreach().

Anyway I think it's still better to fix current behaviour and 
waiting for a decision than leaving this leak open for who knows 
how long...












More information about the Digitalmars-d mailing list