Re: [PATCH 10/10] reftable: make reftable_record a tagged union

Derrick Stolee [off-list ref] writes:

On 12/7/2021 12:45 PM, Han-Wen Nienhuys via GitGitGadget wrote:

quoted

From: Han-Wen Nienhuys <redacted>

This reduces the amount of glue code, because we don't need a void pointer or
vtable within the structure.

The only snag is that reftable_index_record contain a strbuf, so it cannot be
zero-initialized. To address this, introduce reftable_record_for() to create a
fresh instance, given a record type.

Thanks to Peff for the suggestion.

Signed-off-by: Han-Wen Nienhuys <redacted>
---
 reftable/block.c       |   4 +-
 reftable/block_test.c  |  22 +++---
 reftable/generic.c     |  35 ++++----
 reftable/iter.c        |   4 +-
 reftable/merged.c      |  37 ++++-----
 reftable/pq.c          |   3 +-
 reftable/pq_test.c     |  31 ++++----
 reftable/reader.c      | 105 ++++++++++++------------
 reftable/record.c      | 176 ++++++++++++++++-------------------------
 reftable/record.h      |  45 +++++------
 reftable/record_test.c | 162 +++++++++++++++++++------------------
 reftable/writer.c      |  46 ++++++-----

This is a HUGE diff, especially compared to the previous changes
in this series. I recommend splitting this out into its own series
and finding a way to break it down into smaller changes.

As the reftable_record structure is used everywhere (and that is why
this step has to touch everywhere), I suspect that a reviewable fix
in small chunks would be achievable only if we redo the topic that
introduces this hierarchy and fix the type at the source, as if the
reftable_record structure was a struct with union in it from the
beginning, I am afraid.

Perhaps reftable_record_for() can be implemented without changing
the shape of the underlying reftable_record structure in an earlier
step, then all the users of reftable_record instances can be
migrated to call it, and then finally the shape of the structure and
the implementation of reftable_record_for() can be updated?  

If that is doable, then the "migrate each users" part can be split
purely by size.  But (1) I do not know if the first step is even
doable, and (2) I am not sure if it is worth going a somewhat
roundabout route to get to the same destination in this case.

So...