Beware using anonymous methods in loops
Quick, what’s the output of this simple routine?
[code lang="Delphi"] procedure AnonLoop; var i: integer; proc: TProc; ProcList: TList; begin ProcList := TList .Create; for i := 1 to 5 do ProcList.Add( procedure() begin write(i, ' '); end); for proc in ProcList do proc; procList.Free; end; [/code]
Intuitively, you’d think it would write “1 2 3 4 5”, but you’d be wrong. It actually prints out “6 6 6 6 6”. Huh? 6s? You never fed it a 6! And why are they all the same? Well, it has to do with the way anonymous methods work. Take a look at my analysis from a while back and see if it becomes any clearer.
Basically, you aren’t creating a new anonymous method each time you go through the loop. The functor object is created one time only, on the begin line of the enclosing method, not once each time you hit the declaration in the code.
This is a bit disorienting, but technically necessary; if there could be more than one instance of the functor object created each time you run the enclosing method, it would be impossible to support multiple anonymous methods enclosing the same local variables, among other gotchas.
So what to do about it? Seems to me that we have two problems. First, you can’t create multiple anonymous methods by putting a declaration in a loop. Second, the compiler hides this issue from you by allowing the anonymous method to capture the index variable of a for loop, which almost certainly results in silently generating bad code.
To create multiple anonymous methods in a loop, you need to call another function that returns an anonymous method. This can make things a bit difficult, especially since it negates (or at least lessens) the ability of the anonymous method to capture local variables, which is what makes it so useful.
The compiler already can’t capture Result and var/out parameters, because they don’t belong to the current procedure. Seems to me that the next version of the Delphi compiler should similarly check for this, and emit an error if an anonymous method tries to capture the index variable of a for loop, because that almost certainly doesn’t belong to the anonymous method.
BTW I know Barry Kelly reads at least some of my posts, because he occasionally responds with comments here when I write about technical issues. Barry, if you see this, would you mind weighing in on the problem? Thanks.
“Quick, what’s the output of this simple routine?”
Never mind the (interesting, in a classroom/compiler theory way) technical whys and where-fores posed by your post, this trivial case is a perfect example of why anon-methods should be avoided in real world code in the first place.
“Quick, understand this code …” … easy…
First I’ll rewrite it in a way that doesn’t require that my brain not only works like *a* compiler, but works in precisely the way that a *specific* compiler works.
i.e. write it so it can be read by a human, not just a machine.
It may take you 2, 3 or even 5 times longer to create the code, and there may be fare MORE code created in the process, but you will be reap the reward hundreds of times over when you come back to read and/or change that code in the future.
I can not see anything unexpected, this is how anonymous methods work.
It’s valid behaviour because of the capturing the same location.
Try this
procedure AnonLoop;
var
i: integer;
proc: TProc;
ProcList: TList;
function MakeSafeCapture(index:integer):TProc;
begin
result:=procedure()
begin
write(index, ‘ ‘);
end;
end;
begin
ProcList := TList.Create;
for i := 1 to 5 do ProcList.Add(MakeSafeCapture(i));
for proc in ProcList do proc;
procList.Free;
end;
IMHO, an anonymous method should use only its parameters, and no “global” or “local” variables. It sounds to me like bad design, or at least code difficult to follow/maintain.
If you need to access those variables, don’t use an anonymous method, but a regular method.
Or simply provide the current “self” variable to let the anonymous method get its parameters from the “self” properties.
@Sergey Antonov,
I think you mistyped ProcList: TList; instead of ProcList: TList;
and ProcList := TList.Create; with ProcList := TList.Create;
Opps sorry, I think it’s because the formatting of comments (Embarrassing)
@Jolyon
I hope you are not saying that anon-methods should not be used at all. Some task are really quite simpler and more readable using them. I find them especially helpfull when it is important that flow of the code is maintaned and you don’t have to scroll up or down to read the code. Yes I agree they must be use when appropriate and not in all situations. But this is the case with all language constructs.
Take a look at javascript for instance. anon-methods are at the heart of the language, the whole language and its async nature depend on them. And they fit itn naturally. Yes javascript is not Delphi, but I just took this example to show their incredible power if used appropriately.
I’m not sure what surprises you; this is normal for anonymous methods in other languages too.
For instance, the C# code below will print a bunch of lines containing ‘5’:
static void LoopWithAnoymousLambda()
{
List methodList = new List();
for (int i = 0; i
Console.WriteLine(i)
);
}
foreach (Action method in methodList)
{
method();
}
}
–jeroen
Oh nice: your commenting system eats indenting and angle brackets.
Here is the code: https://gist.github.com/709647
–jeroen
I do not get what is “disorienting” about the behavior.
The local variables are used when executing the anonymous method not when the method is stored in the list.
Anything else would be really confusing/strange/wrong.
/Micke
Yes, anonymous methods capture locations, not values. This is what makes them so powerful: that they can act on the locations they have captured. Imagine a ParallelFor method which took an anonymous method as its body; and a data structure you want processed in a parallel way. Rather than have to do handstands with indirection, capturing locations rather than values lets you “pass” the data structure to the body of the ParallelFor method merely by declaring the data structure as a local, and accessing it in the body of the anonymous method as if it were local to that too (and in a nested sense, it is).
@A.Bouchez – if you force yourself to only access parameters in anonymous methods, you’re missing out on what makes them so powerful. Their nested integrated syntax means they are a way of creating new kinds of control flows and abstractions, but in libraries, rather than having to modify the language.
As to other doubters, it’s true that what you didn’t grow up with seems strange at first, but familiarity with the idioms reduces this over time. I believe anonymous methods, by virtue of their relative compactness (I’d prefer to be able to put more type inference in, but can’t yet owing to technical restrictions), are more readable than the alternative, which would have you create a whole separate class, method, constructor to propagate values, lifetime management to take care of the object, etc. All that code would be boilerplate to encode a fairly specific idiom – passing data to an ad-hoc callback, i.e. one that isn’t designed to be reused – and would be obscured because of the other potential uses of that object (people would begin to wonder what else they could use it for when it pops up in code completion, etc).
This is how closures work (and indeed, where the name “closure” comes from). This is standard, and has been so for longer than I have been alive (though not in Delphi, which didn’t exist then). This should work for your example, but I haven’t tested what Delphi does with it:
procedure AnonLoop;
var
..i: integer;
..proc: TProc;
..ProcList: TList;
begin
..ProcList := TList.Create;
..for i := 1 to 5 do
….ProcList.Add(
……procedure()
……var
……..j: Integer
……begin
……..j := i;
……..write(j, ‘ ‘);
……end);
..for proc in ProcList do
….proc;
..procList.Free;
end;
@Jolyon:
I’d have to disagree. Anonymous methods can make your code a lot cleaner in certain situations, most notably in automated testing. The only problem is that — for implementation reasons — Delphi’s anonymous methods are not interchangeble with regular methods. This makes them less useful.
I wrote about it on my blog: http://www.hans-eric.com/2009/06/03/codegear-please-fix-the-anonymous-method-assymetry/
Make sure to read Berry Kelly’s comment for the reasons behind this “assymetry”.
The described behavior is what I would expect, there are no variable states being saved in the list, so when you run through the list again it uses the last state known.
“Basically, you aren’t creating a new anonymous method each time you go through the loop.”
Is this statement actually true, or were you just guessing? As the others have said, the intended behaviour is that ‘i’ is captured, i.e., that ‘i’ is referenced (and kept ‘alive’) each time rather than having its value copied – and that by itself explains the reported behaviour perfectly well.
Hello Mason,
Another observation of your code is that you’re technically accessing the loop index variable *after* the termination of the loop. The value of the loop index variable, per the language spec, is undefined. The fact that it is consistently ‘6’ is merely because that is how the code-gen in this case worked. In other cases, like walking the elements of an array, the look index variable could be converted into a pointer that is incremented by the element size for each iteration. The loop counter could also have been reversed and counted down instead of up had the variable not been referenced within the loop.
Tossing an anonymous method into the mix will likely have the affect of eliminating some potentially available optimizations because the loop variable has been “captured” and moved to a hidden “activation record” which is accessed via an indirection. Captured variables simply *cannot* live on the stack because the anonymous method has enclosed the context.
Yeah, I knew most of that. I probably could have explained the bit about how the loop variable worked, but I figured that part was obvious. :p
That’s part of the reason why I think that it should be a compiler error to access the loop variable inside an anonymous method. First, it indicates that you’re probably trying to do something that won’t work, and second, it violates the “index variable must be a local variable” rule.
BTW it’s nice to learn what “ActRec” stands for. Thanks!
I don’t use anonymous methods too often but my logic tells me that the result is normal, you get 6 because it’s the last value of “i”(the first “for” loop finishes when “i” > 5) before the “for proc in ProcList do”, I wouldn’t expect any other result…
This is expected behavior. And I’m very glad of this. I have implemented ParallelFor using anonymous methods and it works like a charm!
@Hans – yes, and indeed this is an interesting point. Other languages that have anonymous methods NEED them because they do not support functions or procedures as first class entities – they can only exist as methods of a class.
True, anon methods go further even than first class functions, but in Delphi there are simply far fewer situations where anon methods provide anything other than a way of confusing things.
@Barry: That “boiler-plate” code can be key to understanding code when you come to read it and try and understand it later. For sure, once you have committed all the rules to memory, you could deconvolute the compiler trickery “in-line”, but it is still a barrier to comprehension in a way that other compiler “tricks” are not.
i.e. it is useful to know that the compiler is injecting _AddRef and _Release calls around interface references, but whilst this knowledge is useful for delving into deep, dark advanced coding techniques, it is not necessary knowledge in order to be able to simply *read* code written using interfaces in the same way that anon methods requires (deep-er knowledge of the compiler than is immediately apparent in the code in front of you).
It is not a question of “getting used to new idioms”. It is a question of rejecting, rather than embracing, idioms that make life harder in the long run for the sake of things being superficially easier in the short-term.
Oh and for everyone that thinks “Yes, of course this is how it works… what’s the problem” and think that it’s just something to get used to and that there is no scope for misunderstanding…
This blog post is itself evidence of the fallacy of that thinking. 😉