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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Mon, May 06, 2024 at 09:14:55AM -0700, Junio C Hamano wrote:
> 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.

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]