Re: [PATCH v2 6/8] commit-reach: implement ahead_behind() logic

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Mar 10 2023, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
> [...]
> +void ahead_behind(struct commit **commits, size_t commits_nr,
> +		  struct ahead_behind_count *counts, size_t counts_nr)
> +{
> +	struct prio_queue queue = { compare_commits_by_gen_then_commit_date };

I see we have some of this already in this file, so maybe we should
leave it for a subsequent cleanup, but as DEVOPTS=extra-all notes here
we're using a positional initializer.

We could instead be more explicit, and do:

	{ .compare = compare_commits_by_gen_then_commit_date }

But as noted, maybe for asubsequent cleanup...

> +	size_t width = (commits_nr + BITS_IN_EWORD - 1) / BITS_IN_EWORD;
> +	size_t i;

Nit: Consider dropping this "size_t i" line, and instead...

> +
> +	if (!commits_nr || !counts_nr)
> +		return;
> +
> +	for (i = 0; i < counts_nr; i++) {

...just make this "for (size_t i = 0; ...", ditto the 2x ones below.

> +struct ahead_behind_count {
> +	/**
> +	 * As input, the *_index members indicate which positions in
> +	 * the 'tips' array correspond to the tip and base of this
> +	 * comparison.
> +	 */
> +	size_t tip_index;
> +	size_t base_index;
> +
> +	/**
> +	 * These values store the computed counts for each side of the
> +	 * symmetric difference:
> +	 *
> +	 * 'ahead' stores the number of commits reachable from the tip
> +	 * and not reachable from the base.
> +	 *
> +	 * 'behind' stores the number of commits reachable from the base
> +	 * and not reachable from the tip.
> +	 */
> +	unsigned int ahead;
> +	unsigned int behind;

Even though this is the tip of the iceberg in terms of our codebase
overall, can't we just use "size_t" for counts in new APIs?

Trying to squash this into the end-state seems to work:
	
	diff --git a/commit-reach.h b/commit-reach.h
	index 2269fab8261..108651213d9 100644
	--- a/commit-reach.h
	+++ b/commit-reach.h
	@@ -123,8 +123,8 @@ struct ahead_behind_count {
	 	 * 'behind' stores the number of commits reachable from the base
	 	 * and not reachable from the tip.
	 	 */
	-	unsigned int ahead;
	-	unsigned int behind;
	+	size_t ahead;
	+	size_t behind;
	 };
	 
	 /*
	diff --git a/ref-filter.c b/ref-filter.c
	index cdc054beede..b328db696bf 100644
	--- a/ref-filter.c
	+++ b/ref-filter.c
	@@ -2013,7 +2013,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
	 			if (ref->counts) {
	 				const struct ahead_behind_count *count;
	 				count = ref->counts[ahead_behind_atoms++];
	-				v->s = xstrfmt("%d %d", count->ahead, count->behind);
	+				v->s = xstrfmt("%"PRIuMAX" " "%"PRIuMAX, count->ahead, count->behind);
	 			} else {
	 				/* Not a commit. */
	 				v->s = xstrdup("");