Re: [PATCH 0/5] refs: remove functions without ref store

From: Patrick Steinhardt <ps@pks.im>
Date: May 7, 2024, 05:56 UTC
Message-ID: <ZjnCoaVP9_wuYiHh@tanuki>
In-Reply-To: <xmqqseyu3ojk.fsf@gitster.g>

On Mon, May 06, 2024 at 09:14:55AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> > ... But honestly, I doubt
> > that it would be faster for any author of a patch series to figure out
> > that they now need to a define something compared to just adding the
> > `refs_` prefix to their functions.
> 
> The authors would not bother figuring that out while your series is
> not yet in 'master'.  The alternative they have is to base their
> series on top of yours.  You may have "what changes are needed on
> the callers side" in your head, but they don't.

Yeah, that is fair indeed. Theoretically one could re-run Coccinelle whenever merging a new topic. But that would of course put the burden on you, which is something we definitely want to avoid.

Show 6 quoted lines

> Somebody brought up the approach used in <banned.h> to move the
> problem to link time, but in the context the only message we are
> giving is "it is banned - do not use it", which is sufficient over
> there, but probably not in this context.  "it was removed - use this
> instead by adding this suffix and add that as the first parameter"
> is the message I want whoever needs to deal with the fallout to see.

Unfortunately we cannot quite get there at compile time because it is not possible to expand a macro into an error macro with an arbitrary message. The best I could come up with is the following:

    #define REPLACED_REFS_FUNC(func) func ## was_replaced_by_refs_### func

Which results in compiler errors like this:

    bisect.c:712:6: error: use of undeclared identifier 'read_ref_was_replaced_by_refs_read_ref'
      712 |         if (read_ref("BISECT_EXPECTED_REV", &expected_oid))

What is still missing is the bit of informatino that you need to pass in `get_main_ref_store()`. But maybe this is good enough?

Patrick