Re: [PATCH v2 2/5] gdb: prevent prefix commands from being hooks

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-03-09 4:42 a.m., Marco Barisione wrote:
> On 8 Mar 2021, at 21:32, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>> Ahhh, changes that touch command handling are always scary stuff, we
>> need to be really careful not to break existing use cases or paint
>> ourselves in corners for future use cases.  Probably why nobody dares to
>> review it.  I'll give it a try, but know that it still scares me :).
> 
> It did scare me and took a while because of the many corner cases,
> difficult combinations of features.
> 
> My reasoning behind this is that the behaviour is currently confusing
> (that is, what the user would expect doesn’t happen) and forbidding
> this should allow changes in the future quite easily.
> 
>> One question that pops up is, what if someone ever wants to make a hook
>> for a prefixed command.  For example, let's say I want to hook "info
>> inferiors".  I don't think that's possible today, but let's say we want
>> to make it possible.  How would that work?
>>
>> This doesn't work:
>>
>>   (gdb) define hook-info inferiors
>>    Undefined command: "hook-info".  Try "help".
>>
>> So... maybe defining "hook-info" as a prefix command would allow it?
>> Not sure, I didn't really think this through, but I wonder if you
>> thought about that case.
> 
> You can do it with “define info hook-inferiors” which I found a bit
> surprising when I discovered it.

I didn't know that!  And it's even tested:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.base/define.exp;h=ff34e7f294376454fbefb868f4e3f90b6105f0d8;hb=HEAD#l296

... and documented in the manual.

> I think that hooks are incredibly useful but limited. My hope is that
> my patch series will make them less needed as you can get a better and
> more powerful behaviour with command renaming.

I mentioned in a previous review (somewhere in the default-args series,
I can't find it right now) that I was not too comfortable in having
features allowing modifying the behavior of builtin commands in
undocumented ways.  You could say that hooks are a way to do that.  But
at least, even if you define a hook-backtrace, the backtrace part still
works as usual, just that there are some things happening before.  Maybe
I'm a bit too afraid that some script I'm using breaks some other script
I'm using because it decided to rename some core command...

But I see why that would be useful (we use proc renaming in TCL for the
testsuite).  It's a tradeoff, with powerful tools you can go great
things but are also more likely to shoot yourself in the foot.  And I
guess we can't protect the users from themselves for everything,
otherwise our tool is useless.  And since the concept was deemed ok by
other reviewers / maintainers previously, I won't object to it.

Simon