Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Keith Seitz <keiths@redhat.com>
Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [RFA] c++/11734 revisited
Date: Wed, 26 Jan 2011 23:14:00 -0000	[thread overview]
Message-ID: <20110126223526.GA24255@host1.dyn.jankratochvil.net> (raw)
In-Reply-To: <20110124173745.GA31199@host1.dyn.jankratochvil.net>

On Mon, 24 Jan 2011 18:37:45 +0100, Jan Kratochvil wrote:
> > Revised patch attached.
> 
> As a summary:
> 
> "physname" patch I call:
> 	42284fdf9d8cdb20c8e833bdbdb2b56977fea525
> 	http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
> 	dwarf2_physname patchset:
> 	[RFA] dwarf2_physname FINAL
> 	http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html
> 
> Original pre-physname code for pr11734::foo() did not call decode_compound but
> called decode_variable which resolved it from the fully qualified name.
> 
> post-physname code tried to resolve pr11734::foo() via decode_compound which
> did not support such case and it fails now on HEAD.
> 
> Alternatively to your patch the current HEAD can be modified to call
> decode_variable again as in pre-physname era which resolves it correctly but
> for some reason you prefer to resolve pr11734::foo() via decode_compound
> instead which makes sense.

I have to withdraw my preliminary approval.  Together with
	[RFA] c++/12273
	http://sourceware.org/ml/gdb-patches/2010-12/msg00264.html

both patches duplicate the decode_variable functionality by the new
implementation in decode_compound.

But this duplication is still not complete, for example it does not handle
properly namespaces.  It had only a luck the existing namespace tests
(nsusing.exp) do not exploit it.

I have attached such testcase on top of your this patch:
	Re: [RFA] c++/11734 revisited
	http://sourceware.org/ml/gdb-patches/2010-12/msg00263.html

with results:
PASS: gdb.cp/pr11734.exp: break func
PASS: gdb.cp/pr11734.exp: break 'func'
^^^^ = test like nsusing.exp
FAIL: gdb.cp/pr11734.exp: break pr11734::foo() (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo()' (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break pr11734::foo(char*) (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo(char*)' (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break pr11734::foo(int) (got interactive prompt)
FAIL: gdb.cp/pr11734.exp: break 'pr11734::foo(int)' (got interactive prompt)
^^^^ = these funcs get called by the C++ inferior but GDB cannot resolve them.

decode_compound itself should comply to namespacing, just dropping to
decode_variable as before is not enough anyway as decode_variable cannot
resolve class fields which have no global symbols/minsym.

The new plan of G++ parser/plugin is probably not considerable now as a fix.

With the very hacky drop into decode_variable the testcase works but this
patch of mine is sure not usable.



Thanks,
Jan


--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -783,7 +783,7 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Does it look like there actually were two parts?  */
 
-  if (p[0] == ':' || p[0] == '.')
+if (0) //  if (p[0] == ':' || p[0] == '.')
     {
       /* Is it a C++ or Java compound data structure?
 	 The check on p[1] == ':' is capturing the case of "::",
@@ -868,7 +868,22 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
     }
   else
     {
-      p = skip_quoted (*argptr);
+      const char *lang_breakchars;
+      char *breakchars, *s, *end;
+
+      lang_breakchars = current_language->la_word_break_characters ();
+
+      breakchars = alloca (strlen (lang_breakchars) + 1);
+      strcpy (breakchars, lang_breakchars);
+      end = &breakchars[strlen (breakchars)];
+
+      s = strchr (breakchars, ':');
+      if (s != NULL)
+	{
+	  *s = *--end;
+	  *end = 0;
+	}
+      p = skip_quoted_chars (*argptr, NULL, breakchars);
     }
 
   /* Keep any template parameters.  */
--- a/gdb/testsuite/gdb.cp/pr11734-1.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-1.cc
@@ -20,11 +20,21 @@
 
 #include "pr11734.h"
 
+namespace A { void func () {} }
+
+using namespace A;
+
 int
 main ()
 {
   pr11734 *p = new pr11734;
   p->foo ();
+
+  func ();
+
+  pr11734::foo ();
+  pr11734::foo ((char *) "");
+  pr11734::foo (0);
+
   return 0;
 }
-
--- a/gdb/testsuite/gdb.cp/pr11734-2.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-2.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo(void)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734-3.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-3.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo (int a)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734-4.cc
+++ b/gdb/testsuite/gdb.cp/pr11734-4.cc
@@ -20,8 +20,11 @@
 
 #include "pr11734.h"
 
+namespace A {
+
 void
 pr11734::foo (char *a)
 {
 }
 
+}
--- a/gdb/testsuite/gdb.cp/pr11734.exp
+++ b/gdb/testsuite/gdb.cp/pr11734.exp
@@ -34,6 +34,15 @@ if {![runto_main]} {
     continue
 }
 
+if [gdb_breakpoint "func"] {
+  pass "break func"
+}
+if [gdb_breakpoint "'func'"] {
+  pass "break 'func'"
+}
+
+gdb_test "ptype ${class}::l" "type = long"
+
 # An array holding the overload types for the method pr11734::foo.  The
 # first element is the overloaded method parameter.  The second element
 # is the expected source file number, e.g. "pr11734-?.cc".
--- a/gdb/testsuite/gdb.cp/pr11734.h
+++ b/gdb/testsuite/gdb.cp/pr11734.h
@@ -18,11 +18,15 @@
    Please email any bugs, comments, and/or additions to this file to:
    bug-gdb@gnu.org  */
 
+namespace A {
+
 class pr11734
 {
  public:
-  void foo ();
-  void foo (int);
-  void foo (char *);
+  static void foo ();
+  static void foo (int);
+  static void foo (char *);
+  long l;
 };
 
+}


  reply	other threads:[~2011-01-26 22:35 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-09  0:50 Keith Seitz
2010-12-09  4:02 ` Eli Zaretskii
2010-12-09 21:45 ` Tom Tromey
2010-12-09 21:52   ` Jan Kratochvil
2010-12-10 15:21     ` Keith Seitz
2010-12-14 20:03   ` Keith Seitz
2011-01-24 18:15     ` Jan Kratochvil
2011-01-26 23:14       ` Jan Kratochvil [this message]
2011-02-06 22:04     ` Jan Kratochvil
2011-02-06 22:45     ` [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273) Jan Kratochvil
2011-02-08 21:42       ` Tom Tromey
2011-02-10 21:45       ` Keith Seitz
2011-02-17 18:37         ` Keith Seitz
2011-02-18  3:24           ` Keith Seitz
2011-02-21 11:41           ` Jan Kratochvil
2011-02-24 20:41             ` Keith Seitz
2011-02-27 21:18             ` Jan Kratochvil
2011-03-01 22:00               ` Keith Seitz
2011-03-14  7:52                 ` Jan Kratochvil
2011-03-15 19:03                   ` Keith Seitz
2011-03-16  8:28                     ` Jan Kratochvil
2011-03-16 13:58                       ` Tom Tromey
2011-03-16 23:20                       ` Keith Seitz
2011-03-17  3:19                         ` Joel Brobecker
2011-03-17  9:11                           ` Jan Kratochvil
2011-03-17 13:21                             ` Joel Brobecker
2011-02-06 22:46     ` [patch 2/3] Keith's psymtabs fix [Re: [RFA] c++/11734 revisited] Jan Kratochvil
2011-02-06 22:46     ` [patch 3/3] Various linespec fixups " Jan Kratochvil
2011-02-06 22:46     ` [patch 1/3] revert physname part (b) " Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110126223526.GA24255@host1.dyn.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox