This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fwd: Delivery Status Notification (Failure)


[+ keiths, since this is linespec related, and I know
how much he loves linespecs!    :-)]

On Thu, Nov 20, 2014 at 6:57 AM, David Taylor
<dtaylor@usendtaylorx2l.lss.emc.com> wrote:
> Doug Evans <xdje42@gmail.com> wrote:
>> The "," in "-at LOCATION," seems a bit random, relative to other commands.
>>
>> Maybe it is the best way to go.
>> If so, I'd like to see the reasons why it exists documented in the code.
>>
>> Can we just remove the , and require -- when necessary?
>
> It marks the end of the location and the start of the macro.  It is
> patterned after
>
>     maint agent [-at location,] EXPRESSION
> and
>     maint agent-eval [-at location,] EXPRESSION
>
> I did it the same way for consistency.

Ah.  Can't fault that. :-)

Still, I'm more ok with u/i visible hacks in maint commands
than with normal commands.
Bubbling this up into non-maint commands means needing
to now worry about consistency with all the other commands.

> It seems unnecessary, but the location parsing code stops at comma but
> not at space.  Presumably that is so that file names can contain spaces.
> But, that is just a guess on my part.  I personally dislike file names
> containing spaces.

Such names need to be quoted.
OTOH c++ functions with parameters, for example,
don't (usually) need to be quoted.

linespec.c has this:
static const char * const linespec_keywords[] = { "if", "thread", "task" };

This may not be the best solution, and it might involve a few more
changes to linespec.c (or elsewhere) but that's one alternative:
static const char * const linespec_keywords[] = { "if", "thread",
"task", "--" };

It works, but "--" wasn't intended for this purpose (mark the
end of the linespec), so it's just a not-well-thought-out-idea.

I tried to think of a situation where using a comma here would
lead to u/i warts later, but couldn't.
E.g., if the comma gets used in other contexts
then will users start complaining that "b foo, thread 1" doesn't work?
If that day comes we *could* allow the comma in that context.
But it would be odd that a comma is *required* in some
contexts and not in others.

In the end, I'm ok with the patch, but
I think the docs (both offline and online) need to highlight the comma
as being a requirement for "-at" since it's not intuitive.
Plus I think we need to document in some linespec-related
place that "," is now required to be recognized as ending a linespec.
I don't know if that exists today.
[FAOD, a "," within a linespec is still ok,
e.g., "info macro -at foo(int, int), BAR"]
Ok with those changes.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]