Re: [PATCH 0/6] remove USE_THE_INDEX_COMPATIBILITY_MACROS

[Phillip Wood <phillip.wood123@gmail.com>]

On 19/12/2022 15:11, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Dec 19 2022, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 15/12/2022 09:58, Ævar Arnfjörð Bjarmason wrote:
>>> My recent now-landed topic[1] to remove most use of
>>> "USE_THE_INDEX_COMPATIBILITY_MACROS" was merged in 041df69edd3 (Merge
>>> branch 'ab/fewer-the-index-macros', 2022-11-28).
>>> It left out use of the macros that would have conflicted with
>>> in-flight changes, but as those topics have landed we can now complete
>>> the migration.
>>> As before this is almost entirely a matter of applying the existing
>>> "pending" coccinelle rules, the exceptions being 1/6, and the *.h
>>> changes where we remove the macro definitions (the macro users being
>>> edited by coccinelle).
>>> The 4-5/6 then handle some edge cases we had left (but the code
>>> change
>>> itself is done by coccinelle).
>>
>> I've only given these patches a quick scan, but I think they look
>> good. None of the callers that are converted here are in library code
>> so using the_index makes perfect sense.
> 
> Thanks for the review.
> 
> That's correct, although even if that were the case that wouldn't be an
> issue with this migration, as we'd have been using "the_index" before,
> just indirectly through a macro.

Indeed, I'm just not convinced that it is worth removing the macro in 
library code without changing to take a struct istate (I don't see the 
existence of the macro itself as a problem as I think it is just a 
symptom of the real problem) but I seem to be in the minority on that point.

Best Wishes

Phillip

> That wasn't the case here, but I do I have another similar migration for
> migrating "the_repository" wrappers.
> 
> In those cases there's surely instances where e.g. we really should be
> using a "r" argument instead, but I've opted to leave that question out,
> as it would make the coccinelle rules involved & diffs much harder to
> deal with.
> 
> And because in the end the result is the same if viewed with "cc -E",
> i.e. these are just the macro shims we've been meaning to stop using for
> a while.
>