Re: [PATCH v2 3/3] ref-filter: populate symref from iterator

John Cai [off-list ref] writes:

Yes that's a good question. I took a look and it seems like in *most* cases by
the time populate_value() is called, apply_ref_filter() has already been called that
populates the symref member of ref_array_item.

populate_value() gets called by get_ref_atom_value() which gets called by both

1. format_ref_array_item()
2. cmp_ref_sorting()

In the case of [2], the callchain starts with filter_and_format_refs() which
calls ref_array_sort() that eventually calls populate_value(). Before
ref_array_sort() is called, filter_refs() is called which ends up calling
do_filter_refs() with filter_one(), leading to apply_ref_filter().

In the case of [1] however, there are a couple of code paths that call
populate_value() without apply_ref_filter() ever being called.

pretty_print_ref() directly calls format_ref_array_item() ->
get_ref_atom_value() -> populate_value(). However, apply_ref_filter() is not
called which means the symref will not be populated.

I am perfectly OK to keep the "fallback" code, even if you haven't
found a concrete reproduction to trigger it.  Removing it is not
part of what this series is trying to do, anyway.  The theme of the
topic has always been "optimize when we can", at least to me.

Having said that, if we somehow can give the analysis you did above
as hint to future developers, then they can start from there when
they consider if they can remove the "fallback" code.  It would help
even it just said

    /*
     * NEEDSWORK: this might have become a dead code after
     * optimization to grab symref target in apply_ref_filter().
     */

and nothing else.