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

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Sep 24, 2025, 15:58 UTC
Message-ID: <CAH=ZcbAcvoXJYPHE=mcZSqPLuUPxKcSs-vd307X7uXi+aG3evw@mail.gmail.com>
In-Reply-To: <xmqqqzvvn9tq.fsf@gitster.g>

On Wed, Sep 24, 2025 at 9:34 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 37 quoted lines

>
> Ezekiel Newren <ezekielnewren@gmail.com> writes:
>
> > On Wed, Sep 24, 2025 at 4:21 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> On 23/09/2025 22:24, Ezekiel Newren via GitGitGadget wrote:
> >> > From: Ezekiel Newren <ezekielnewren@gmail.com>
> >>
> >> The commit message should explain why this change is being made
> >
> > Reasons to delete local variable aliasing:
> >   * Usage tracking: Tools are better able to follow the usage.
> >   * Refactor churn: Later commits will refactor rchg.
> >   * No additional meaning: The local variables express the same meaning
> >     as the struct field itself.
> >
> > Would that suffice?
>
> In general, I do not view the first one is a good excuse.
>
> When using a separate local variable enhances readability of the
> code (which often is true, with a pointer that points deep into a
> nested structure member) to humans, we shouldn't blindly bend the
> code to cater to less intelligent tools; it needs balancing.
>
> The third one alone is not a good excuse for the same reason.  It
> (and the first one) depends on how much benefit we are gaining from
> having a short-and-sweet local variables that may make the expressions
> and statements they are involved in easier to read.
>
> For this particular change, I would think it is borderline, and
> subjective.  I would be OK with the third point if you rephrase it
> to additionally say that the conditional and the inner loop is easy
> enough to follow without using the local aliases to make the code
> shorter (which of course is the commit author's opinion, but they
> deserve to have and express their opinion as part of the rationale
> for a change).

Ok, I can see that.

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... 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. Since C doesn't have a concept of borrowing, I decided to delete the aliasing here, and in many other places. I'm removing as much aliasing from the code as I can because it makes it easier to translate into idiomatic Rust later. Using ctags and modern IDE's to follow variable usage is convenient, but safe Rust refuses to compile in many cases where C aliasing is common. We could use unsafe Rust to make literal translations of C to Rust, but then we'd forfeit the reasons and benefits of why we want to add Rust to Git in the first place.

Translating C to Rust has been difficult because many styles in Git's C flat out won't compile in Rust. Many places need a little tweaking, others need major overhauls. In all of my C cleanups I am keeping idiomatic Rust in mind.