Re: [PATCH 1/5] Introduces for_each_revision() helper

["Shawn O. Pearce" <spearce@spearce.org>]

Junio C Hamano <junkio@cox.net> wrote:
> The reason I do not like this particular one is because both
> operations you are hiding are not simple operations like
> "initialize a variable to list head" or "follow a single pointer
> in the structure", but rather heavyweight operations with rather
> complex semantics.  I would want to make sure that people
> realize they are calling something heavyweight when they use the
> revision traversal.

But in_merge_base is heavyweight if the two commits are in the
same object database, but aren't connected at all.  You'll need
to traverse both histories before aborting and saying there is
no merge base.  That ain't cheap on large trees.  But its also a
single line of code.

Anyway, my original problem with this macro was the way it was
defined.  I think Luiz was able to fix most of my issues with it
in his latest version, but I still have a personal distaste for
hiding things like a for(;;) construct in a macro, or allowing a
macro parameter to be used more than once within the definition of
the macro (unexpected side-effects of evaluating an more than once).

-- 
Shawn.