Re: [expr] [patch] Fix "break expr if (cond)" regression

[Keith Seitz <keiths@redhat.com>]

On 01/21/2010 08:40 AM, Jan Kratochvil wrote:
> commit 97a9a53e60460dd246de6a1d5ce50bf3497a1eb8
> Author: keiths<keiths@redhat.com>
> Date:   Thu Jan 14 18:51:45 2010 -0800
>
>          * linespec.c (decode_line_1): Keep overload information for the
>          non-compound case.

Grr... Linespecs again. Once you get into them, they never let you go.

>   (gdb) break marker1 if (1==1)
> -Breakpoint 4 at 0x80485eb: file gdb/testsuite/gdb.base/break1.c, line 45.
> -PASS: gdb.base/condbreak.exp: break marker1 if (1==1)
> +Function "marker1 if (1==1)" not defined.
> +Make breakpoint pending on future shared library load? (y or [n]) n
> +FAIL: gdb.base/condbreak.exp: break marker1 if (1==1) (got interactive prompt)

Ugh! My bad. On my last commit, I checked in something dubious: the call 
to strrchr -- I originally wrote something a little more elaborate to do 
that job. Now I remember why I originally did that. I reverted that bit 
of the change locally, and that fixes all the regressions.

Here's the patch that I pulled from the history:

diff --git a/gdb/linespec.c b/gdb/linespec.c
index d6f0ed8..fdd7b5a 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -820,9 +820,25 @@ decode_line_1 (char **argptr, int funfirstline, 
struct symtab *default_symtab,
      p = find_template_name_end (p);

    /* Keep method overload information.  */
-  q = strchr (p, '(');
-  if (q != NULL)
-    p = strrchr (q, ')') + 1;
+  if (*p == '(')
+    {
+      int depth = 0;
+
+      while (*p)
+	{
+	  if (*p == '(')
+	    ++depth;
+	  else if (*p == ')')
+	    {
+	      if (--depth == 0)
+		{
+		  ++p;
+		  break;
+		}
+	    }
+	  ++p;
+	}
+    }

    /* Make sure we keep important kewords like "const" */
    if (strncmp (p, " const", 6) == 0)

> I do not fully agree with my patch but I also do not fully agree with the
> former patch, maybe bison should be used to parse the expression, but rather
> someone (Tom? unaware) said that "break E [thread T] [if C]" should be rather
> changed to the more parseable+standardized "break [-thread T] [-if C] E" so
> that these expression parsing difficulties would be present only as a backward
> compatibility and do not need to supersede the former functionality.

Yeah, it would be wonderful, in general, to get rid of linespecs 
entirely. They suck big time.

Sorry about that.

Keith