[RFC/RFA] Add handling for unqualified Ada operators in linespecs

[RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Joel Brobecker]

Hello,

This patch enhances the linespec parser to recognize unqualified
operator names in linespecs. This allows the user to insert a breakpoint
on operator "+" as follow, for instance:

        (gdb) break "+"

Previously, it was possible to insert such a breakpoint, but one
had to fully qualify the function name. For instance:

        (gdb) break ops."+"

Not the most elegant solution, but relatively self contained. So
I thought I'd submit it, after all.  It's still not completely
functional because, for it to work, it needs the symtabs to be
already read-in (or, in other words, the partial symbol search is
still not working).  But that's actually another, much more general
problem, which is related to breakpoints using unqualified function
names in general.  I will try to think about that on its own.

gdb/ChangeLog:

        * linespec.c (locate_first_half): Add handling of Ada operators
        when the current language is Ada.

Tested on x86_64-linux. It should also only affect Ada.
OK to apply?

---
 gdb/linespec.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 0ac54f7..2201be3 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1334,6 +1334,24 @@ locate_first_half (char **argptr, int *is_quote_enclosed)
   char *p, *p1;
   int has_comma;
 
+  /* Check if the linespec starts with an Ada operator (such as "+",
+     or ">", for instance).  */
+  p = *argptr;
+  if (p[0] == '"'
+      && current_language->la_language == language_ada)
+    {
+      const struct ada_opname_map *op;
+
+      for (op = ada_opname_table; op->encoded != NULL; op++)
+        if (strncmp (op->decoded, p, strlen (op->decoded)) == 0)
+	  break;
+      if (op->encoded != NULL)
+	{
+	  *is_quote_enclosed = 0;
+	  return p + strlen (op->decoded);
+	}
+    }
+
   /* Maybe we were called with a line range FILENAME:LINENUM,FILENAME:LINENUM
      and we must isolate the first half.  Outer layers will call again later
      for the second half.
-- 
1.7.1

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This patch enhances the linespec parser to recognize unqualified
Joel> operator names in linespecs. This allows the user to insert a breakpoint
Joel> on operator "+" as follow, for instance:
Joel>         (gdb) break "+"

I think it is fine to allow this.

I wonder whether this requires a documentation change.

Joel> Not the most elegant solution, but relatively self contained. So
Joel> I thought I'd submit it, after all.  It's still not completely
Joel> functional because, for it to work, it needs the symtabs to be
Joel> already read-in (or, in other words, the partial symbol search is
Joel> still not working).  But that's actually another, much more general
Joel> problem, which is related to breakpoints using unqualified function
Joel> names in general.  I will try to think about that on its own.

I think we should continue trying to look at solutions to linespec and
symbol table problems in as language-independent a way as possible.
And, if we can't be language-independent, we should endeavor to have
clean code; the current stuff is still a big mess.

I think Keith's changes will help some here...

Joel> +  p = *argptr;
Joel> +  if (p[0] == '"'

Why only double quotes?

I don't even mind that as a temporary measure.  But I think (or hope)
Keith's plan is for single- and double-quotes to be considered
identically, and uniformly, throughout linespec.

Tom

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Doug Evans]

On Fri, Dec 16, 2011 at 11:29 AM, Tom Tromey <tromey@redhat.com> wrote:
> I don't even mind that as a temporary measure.  But I think (or hope)
> Keith's plan is for single- and double-quotes to be considered
> identically, and uniformly, throughout linespec.

+1

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Keith Seitz]

On 12/16/2011 11:29 AM, Tom Tromey wrote:
> I don't even mind that as a temporary measure.  But I think (or hope)
> Keith's plan is for single- and double-quotes to be considered
> identically, and uniformly, throughout linespec.

Wanting to simplify this as much as possible, I eliminated the 
double-quote. The choice was rather arbitrary, and it can be added or 
changed rather easily.

Keith

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> On 12/16/2011 11:29 AM, Tom Tromey wrote:
>> I don't even mind that as a temporary measure.  But I think (or hope)
>> Keith's plan is for single- and double-quotes to be considered
>> identically, and uniformly, throughout linespec.

Keith> Wanting to simplify this as much as possible, I eliminated the
Keith> double-quote. The choice was rather arbitrary, and it can be added or
Keith> changed rather easily.

It seems like this change would break some actually existing user
linespecs.

Tom

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Joel Brobecker]

Hi Tom

Thanks for the review!

> I think we should continue trying to look at solutions to linespec and
> symbol table problems in as language-independent a way as possible.
> And, if we can't be language-independent, we should endeavor to have
> clean code; the current stuff is still a big mess.

I agree. I haven't had much time to think about this, but I think
it might be quite a challenging project. I need to get up to speed
on Keith's project...

> Joel> +  p = *argptr;
> Joel> +  if (p[0] == '"'
> 
> Why only double quotes?

Because the double quotes are part of the operator name... Eg:

    function "+" (A, B: My_Type) return My_Type;

And if we wanted to be pedantic, the debugger should also accept:

    (gdb) break '"+"'

By comparison, '+' is an entirely different thing (the character +),
and thus breaking on '+' should be rejected.

That being said, if it helps Keith's work to treat them similarly,
then I don't mind being a little approximate here, and treat both
quote characters as the equivalent for linespec purposes. I doubt
that any Ada programer would ever write...

    (gdb) break '+'

... and if that were the case, that he'd complain much about it
inserting a breakpoint on operator "+".

-- 
Joel

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> +  p = *argptr;
Joel> +  if (p[0] == '"'
>> 
>> Why only double quotes?

Joel> Because the double quotes are part of the operator name... Eg:

Joel>     function "+" (A, B: My_Type) return My_Type;

I think this could mess with the lexing plans.  The original plan was
for quotes to mean quoting.  This approach means there is a situation
where the quotes are part of the name.

Keith... ?

Tom

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Joel Brobecker]

> Joel> Because the double quotes are part of the operator name... Eg:
> 
> Joel>     function "+" (A, B: My_Type) return My_Type;
> 
> I think this could mess with the lexing plans.  The original plan was
> for quotes to mean quoting.  This approach means there is a situation
> where the quotes are part of the name.
> 
> Keith... ?

If it makes thing simpler for the debugger, we could consider
the idea of accepting just + as the function name, without the quotes,
and consider the quotes as linespec quotes rather than part of
the function name. It would mean that GDB would end up accepting
the following 3 forms instead of just the first one:

    break "+"
    break '+'
    break +

But that would be OK. In Ada, '+' is really the character '+',
so it would normally make no sense in a linespec.  But on the
other hand, since it makes no sense at all, we can assume that
no one in their right mind will ever use it, and if they do,
then call it a feature (because there is no other sensible thing
to do anyway).

It is worth mentioning the fact that `break +' currently assumes
that the `+' starts an offset, rather than indicates an operator
name. Surprisingly, I would have expected this to be equilvalent to
`break +0', whereas it seems to be equivalent to `break +5'!!!
Oh, not documented in the GDB manual, but seems definitely
intentional - see decode_all_digits:

    case plus:
      if (q == *argptr)
        val.line = 5;

-- 
Joel

Re: [RFC/RFA] Add handling for unqualified Ada operators in linespecs

[Joel Brobecker]

> Joel> This patch enhances the linespec parser to recognize unqualified
> Joel> operator names in linespecs. This allows the user to insert a breakpoint
> Joel> on operator "+" as follow, for instance:
> Joel>         (gdb) break "+"
> 
> I think it is fine to allow this.

Thanks! I have committed this patch.

> I wonder whether this requires a documentation change.

Given that from an Ada programer's perpective, there is nothing special
about this name, so I do not think so. I checked the Ada part of the
manual, and it didn't say that we were not supporting these breakpoints.
So I think we're good.

As stated earlier, I am open to further adjustments if it makes
it easier to deal with the quotes...  This could prepare the ground
before Keith's patch comes in, rather than after. I just don't know
enough of Keith's patch yet to be sure which approach to take.

-- 
Joel