From: Kristofer Karlsson Date: Fri, 26 Jun 2026 21:57:37 GMT Subject: Re: [PATCH v3 5/8] commit-reach: introduce struct paint_state with per-side counters Message-ID: In-Reply-To: On Fri, 26 Jun 2026 at 23:13, René Scharfe wrote: > > > +struct paint_state { > > + struct prio_queue queue; > > + int p1_count; > > + int p2_count; > > + int pending_merge_bases; > > +}; > Can they become negative? Wouldn't size_t be a more natural fit, > matching nr from struct prio_queue? Negative would be a clear indication of a bug though that's not checked right now anyway. And since it's not checked we might as well use size_t instead - and it would technically be more correct though I struggle to imagine a case where the number of active elements in the frontier exceeds 2^31 or whatever a signed int would give. I am happy to change to size_t. > And some bikeshedding: > > Why abbreviate? parent1_count and parent2_count would be slightly > easier to read and associate with PARENT1 and PARENT2. > > And pending_merge_bases is a counter as well. Why not call it > like that, pending_merge_base_count? Well, that's pretty long. > both_count? That's quite generic and nondescript. Call the other > counters parents1 and parents2? Nah. Or parent1s and parent2s? > Not sure why this inconsistency bothers me to begin with. Fair point, I was thinking that the surrounding context is so small that the naming almost doesn't matter - the terms don't escape paint_down_to_common. I am happy to change to something like: parent1_count, parent2_count, mb_candidate_count to make it more consistent. It seems the mb_ prefix is already used for merge bases in some files - best example is perhaps builtin/diff.c I see in the codebase that we are using multiple styles, perhaps depending on specific context. - nr_ prefix: nr_objects, nr_paths_watching - num_ prefix: num_commits, num_hashes, num_workers - _count suffix: entry_count, max_count, skip_count so I think _count suffix is a good choice at least - it matches other usages where we typically just increment or decrement. Thanks, Kristofer