[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 Andrej Mitrovic changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #10 from Andrej Mitrovic 2011-07-11 08:27:55 PDT --- Fixed in 2.054 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #9 from Andrej Mitrovic 2011-05-24 17:11:28 PDT --- Btw, I think I know what you were referring to now. The opApply at line 1984 uses a delegate that takes a ref string. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #8 from Steven Schveighoffer 2011-05-24 17:08:24 PDT --- I think this was a D1 module "casted" to D2 to get it to compile :) Probably would be better served reimplemented, but obviously someone would have to spend some quality time doing that. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #7 from Andrej Mitrovic 2011-05-24 16:57:25 PDT --- Actually there's 4 opApply implementations in that module and each of them is casting char[] to a string. And other methods like getKeyName, getValueName, etc.. seem to do the same thing. Ugly! -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #6 from Andrej Mitrovic 2011-05-24 16:53:41 PDT --- Yeah from a quick glance at the registry module it does seem a little fishy overall. __gshared variables, ASCII-only WinAPI calls (what if the registry key is in UTF, if that's possible?), casts to strings.. maybe this module needs to be refactored. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #5 from Steven Schveighoffer 2011-05-24 15:28:28 PDT --- I completely misunderstood the problem here, I thought the string was being directly returned in the opApply delegate, not a Key object. Yes, I agree, the string should be idup'd. Doing anything differently would require a redesign (which I think actually is in order, to allocate that much data to do a simple loop is incredibly wasteful), but since nobody is owning this, your solution is as good as we can get for now. Sorry for the noise. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059
--- Comment #4 from Andrej Mitrovic 2011-05-24
15:12:35 PDT ---
Change what exactly to const(char[])? You probably mean my "name" variable in
the example code.
I think this is again that issue of implicitly reusing a variable in a foreach
loop. I really wish this was more explicit instead of relying on documentation,
e.g. I'd love it if it were like this:
foreach (ref buffer; iterator()) { } // it's reusing buffer on each iteration
foreach (buffer; iterator()) { } // allocates new buffer on each iteration
Maybe it's easy to reason about the current foreach mechanism when you only
have one foreach loop. But when you have multiple foreach statements
intertwined with dozens of different lines it's easy to miss that you should
have duped something.
It took me a good while before I realized what was wrong with my code until I
ran it down to a simple test case like in the original post. I simply thought
"hey, I'm assigning an immutable string to an immutable string, what could go
wrong?".
But this is more worthy of a post in the NG then to discuss it in here I
guess..
--
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #3 from Steven Schveighoffer 2011-05-24 14:44:26 PDT --- I think the better solution is to change it to const(char)[]. This better reflects that the data may be changing between iterations. If you are not storing the strings from this, then it is extremely wasteful to allocate a new memory block for each iteration. You can always idup or dup if you want to. Note, I think the documentation needs to specifically state it reuses the buffer, so you should .dup or .idup if you wish to keep the data beyond the loop iteration. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 --- Comment #2 from Andrej Mitrovic 2011-05-24 14:36:34 PDT --- Should I just change that line to: Key key = m_key.getKey(sName[0 .. cchName].idup); I think this makes the most sense if we're to trust that strings are immutable. I can make a pull request if this fix would be ok (my tests show that this fixes the issue). -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 5059] String assignment in foreach loop breaks immutability
http://d.puremagic.com/issues/show_bug.cgi?id=5059 Jesse Phillips changed: What|Removed |Added CC||jesse.k.phillip...@gmail.co ||m --- Comment #1 from Jesse Phillips 2011-04-28 10:13:09 PDT --- This is a bug in KeySequence's opApply. The function is reusing a char[] for every iteration, it then casts this to a string, but ends up being modified for each call to Reg_EnumKeyName_. https://github.com/D-Programming-Language/phobos/blob/master/std/windows/registry.d#L1870 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
