Re: target-delegates.c needs some TLC [was Re: [OB PATCH] target.h (to_traceframe_info): Fix TARGET_DEFAULT_RETURN]

Pedro Alves
2014-03-07T18:33:56Z

On 03/06/2014 05:20 AM, Doug Evans wrote:
> On Mon, Mar 3, 2014 at 5:36 PM, Yao Qi <yao@codesourcery.com> wrote:
>> On 03/04/2014 09:18 AM, Hui Zhu wrote:
>>> I cannot understand about this OB is not right. I have 2 questions to you:
>>> 1. Before my patch, does target-delegates.c that generated by make-target-delegates is same with current target-delegates.c?
>>
>> No, as I said, I forgot to re-generate target-delegates.c.
>
> Hmmm....
>
> I don't even see target-delegates.c in Makefile.in. That feels like a
> bug. [Could be blind of course. :-)]
> I realize there's a comment in target-delegates.c that says how to
> regenerate it, but these kind of things are part of what makefiles are
> for.

This also crossed my mind when initially reviewing the series (and
I'm sure Tom's too when writing it, as it's such an obvious thing),
but realized this is really no different from e.g., gdbarch.h|c.

So given the precedent, I don't consider this a particular bug of
target-delegates.c, but a more generic "IWBN if we had Makefile
rules for our generated files".

Of course, I'd welcome patches in that direction.

> I'm not sure I'd want to require perl for --enable-maintainer-mode
> (which is a common trigger for enabling in makefiles the appropriate
> rules to auto-regenerate checked-in machine-generated files), but it's
> one thought.

I don't see a problem there. automake is perl as well, for instance,
and it's common for --enable-maintainer-mode to trigger automake/aclocal.

Even if that weren't true, by configuring with --enable-maintainer-mode,
by definition you're asserting you have the tools required for
regular gdb maintenance, and given make-target-delegates is perl,
well, any maintainer must have it handy.

--
Pedro Alves