Re: [PATCH v5 10/13] xdiff: delete rchg aliasing

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Sep 24, 2025, 22:46 UTC
Message-ID: <CAH=ZcbAm8++qGNX883epk0h3NA-Z=8+RX1bDJeTe=n2jjSsdmg@mail.gmail.com>
In-Reply-To: <xmqqikh7k05r.fsf@gitster.g>

On Wed, Sep 24, 2025 at 3:31 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

>
> Ezekiel Newren <ezekielnewren@gmail.com> writes:
>
> > I have a question for everyone: Does preparing C code to be translated
> > into Rust count as a valid reason for changing it? Provided that there
> > is no violation of the Git style (or very small in some cases).
> >
> > If my intent was to keep this as C code forever I'd agree, but...
>
> You'd agree that "I am preparing this for eventual rewrite" would be
> a valid reason?  Or you are agreeing with something else?

I'd agree that my reasons for making this change are insufficient. I think usage tracking tools _is_ a weak argument, but perhaps not quite as weak as what you're thinking. For example, when I renamed the rchg field to changed, it was as simple as right-clicking the field, choosing Rename, typing 'changed', and letting the IDE update every use. Patch 11/13, "xdiff: rename rchg -> changed in xdfile_t", was generated directly from that one action. That patch was clean because I had already gone through and removed all the aliases of that field.

Show 10 quoted lines

> > My
> > other reason is that it more closely follows Rust paradigms. Creating
> > multiple pointers to the same memory in Rust subverts the borrow
> > checker's ability to keep track of who owns the memory.
>
> Sure.  But looking at the use of rchg[12] in xdl_build_script(), if
> they were "const char *", combined with the fact that they are local
> and their addresses are never taken (to be leaked to our callers),
> you wouldn't have much trouble with the current code, or would you
> still have issues?

For xdl_build_script() specifically it would work just fine keeping the local variable aliasing in. I think this is another case of personal preference vs established style. Which path would you prefer that I take?

1. Drop this commit and remember to refactor rchg1, rchg2 to changed1,
and changed2.
2. Keep this commit with reasons like this:
  * Refactor churn: Later commits will refactor rchg.
  * No additional meaning: The local variables express the same
meaning as the struct field itself. Also, the conditional and the
inner loop is easy enough to follow without using the local aliases to
make the code shorter.

My preference is number 2.