* RFA: fix PR gdb/2489
@ 2008-08-01 19:15 Tom Tromey
2008-10-21 22:25 ` Pedro Alves
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2008-08-01 19:15 UTC (permalink / raw)
To: gdb-patches
This patch fixes PR gdb/2489.
The bug is that field name completion does not consider methods.
The fix is to also examine method names when completing; but not to
consider constructor names.
Built and regression tested on x86-64 (compile farm).
New test case included.
Ok?
Tom
b/gdb/ChangeLog:
2008-08-01 Tom Tromey <tromey@redhat.com>
PR gdb/2489:
* completer.c (count_struct_fields): Count method names.
(add_struct_fields): Add matching method names.
b/gdb/testsuite/ChangeLog:
2008-08-01 Tom Tromey <tromey@redhat.com>
* gdb.cp/pr2489.cc: New file.
* gdb.cp/cpcompletion.exp: New file.
diff --git a/gdb/completer.c b/gdb/completer.c
index e7ee817..e42d8a3 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -339,7 +339,7 @@ location_completer (char *text, char *word)
}
/* Helper for expression_completer which recursively counts the number
- of named fields in a structure or union type. */
+ of named fields and methods in a structure or union type. */
static int
count_struct_fields (struct type *type)
{
@@ -353,17 +353,25 @@ count_struct_fields (struct type *type)
else if (TYPE_FIELD_NAME (type, i))
++result;
}
+
+ for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
+ {
+ if (TYPE_FN_FIELDLIST_NAME (type, i))
+ ++result;
+ }
+
return result;
}
-/* Helper for expression_completer which recursively adds field names
- from TYPE, a struct or union type, to the array OUTPUT. This
- function assumes that OUTPUT is correctly-sized. */
+/* Helper for expression_completer which recursively adds field and
+ method names from TYPE, a struct or union type, to the array
+ OUTPUT. This function assumes that OUTPUT is correctly-sized. */
static void
add_struct_fields (struct type *type, int *nextp, char **output,
char *fieldname, int namelen)
{
int i;
+ char *type_name = NULL;
CHECK_TYPEDEF (type);
for (i = 0; i < TYPE_NFIELDS (type); ++i)
@@ -378,6 +386,22 @@ add_struct_fields (struct type *type, int *nextp, char **output,
++*nextp;
}
}
+
+ for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
+ {
+ char *name = TYPE_FN_FIELDLIST_NAME (type, i);
+ if (name && ! strncmp (name, fieldname, namelen))
+ {
+ if (!type_name)
+ type_name = type_name_no_tag (type);
+ /* Omit constructors from the completion list. */
+ if (strcmp (type_name, name))
+ {
+ output[*nextp] = xstrdup (name);
+ ++*nextp;
+ }
+ }
+ }
}
/* Complete on expressions. Often this means completing on symbol
diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
new file mode 100644
index 0000000..1f9caa6
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
@@ -0,0 +1,78 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@prep.ai.mit.edu
+
+# This file is part of the gdb testsuite.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile pr2489
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile "${srcdir}/${subdir}/${testfile}.cc" "${testfile}.o" object {c++ debug}] != ""} {
+ untested completion.exp
+ return -1
+}
+
+if {[gdb_compile "${testfile}.o" ${binfile} executable {c++ debug}] != "" } {
+ untested completion.exp
+ return -1
+}
+
+gdb_exit
+
+# Don't let a .inputrc file or an existing setting of INPUTRC mess up
+# the test results. Even if /dev/null doesn't exist on the particular
+# platform, the readline library will use the default setting just by
+# failing to open the file. OTOH, opening /dev/null successfully will
+# also result in the default settings being used since nothing will be
+# read from this file.
+global env
+if [info exists env(INPUTRC)] {
+ set old_inputrc $env(INPUTRC)
+}
+set env(INPUTRC) "/dev/null"
+
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "Set breakpoint here" ${testfile}.cc]
+
+if {![runto $bp_location]} {
+ perror "test suppressed"
+}
+
+send_gdb "p foo1.g\t"
+gdb_expect {
+ -re "^p foo1\\.get_foo $"\
+ { send_gdb "()\n"
+ gdb_expect {
+ -re "^.* = 0.*$gdb_prompt $"\
+ { pass "complete 'p foo1.g'"}
+ -re ".*$gdb_prompt $" { fail "complete 'p foo1.g'"}
+ timeout {fail "(timeout) complete 'p foo1.g'"}
+ }
+ }
+ -re ".*$gdb_prompt $" { fail "complete 'p foo1.g'" }
+ timeout { fail "(timeout) complete 'p foo1.g' 2" }
+ }
+
diff --git a/gdb/testsuite/gdb.cp/pr2489.cc b/gdb/testsuite/gdb.cp/pr2489.cc
new file mode 100644
index 0000000..f735205
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pr2489.cc
@@ -0,0 +1,34 @@
+
+class Foo
+{
+
+private:
+ int foo_value;
+
+public:
+ Foo () { foo_value = 0;}
+ Foo (int i) { foo_value = i;}
+ ~Foo () { }
+ void set_foo (int value);
+ int get_foo ();
+
+ bool operator== (const Foo &other) { return foo_value == other.foo_value; }
+};
+
+void Foo::set_foo (int value)
+{
+ foo_value = value;
+}
+
+int Foo::get_foo ()
+{
+ return foo_value;
+}
+
+int main ()
+{
+ Foo foo1;
+ foo1.set_foo (42); // Set breakpoint here.
+ return 0;
+}
+
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2008-08-01 19:15 RFA: fix PR gdb/2489 Tom Tromey
@ 2008-10-21 22:25 ` Pedro Alves
2009-02-03 1:02 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2008-10-21 22:25 UTC (permalink / raw)
To: gdb-patches, tromey
On Friday 01 August 2008 20:14:50, Tom Tromey wrote:
> This patch fixes PR gdb/2489.
>
> The bug is that field name completion does not consider methods.
>
> The fix is to also examine method names when completing; but not to
> consider constructor names.
>
> b/gdb/ChangeLog:
> 2008-08-01 Tom Tromey <tromey@redhat.com>
>
> PR gdb/2489:
> * completer.c (count_struct_fields): Count method names.
> (add_struct_fields): Add matching method names.
>
> b/gdb/testsuite/ChangeLog:
> 2008-08-01 Tom Tromey <tromey@redhat.com>
>
> * gdb.cp/pr2489.cc: New file.
> * gdb.cp/cpcompletion.exp: New file.
>
This looks mostly OK to me. This patch is pending for a few months
now, so it shouldn't hurt to wait a couple of days more to give the
C++ maintainer a chance to comment. :-)
> + for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
^ missing space
We have this description in gdbtypes.c:
/* Return a typename for a struct/union/enum type without "struct ",
"union ", or "enum ". If the type has a NULL name, return NULL. */
char *
type_name_no_tag (const struct type *type)
{
+ for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
+ {
+ char *name = TYPE_FN_FIELDLIST_NAME (type, i);
+ if (name && ! strncmp (name, fieldname, namelen))
+ {
+ if (!type_name)
+ type_name = type_name_no_tag (type);
+ /* Omit constructors from the completion list. */
+ if (strcmp (type_name, name))
+ {
Can type_name ever be NULL here then? I'm no guru in this
area, but I think not.
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@prep.ai.mit.edu
Please remove this bit. Jan Kratochvil has recently gone through removing
all references to this long gone address. (It seems a couple have crept
in since though.)
> +
> +if {[gdb_compile "${srcdir}/${subdir}/${testfile}.cc" "${testfile}.o" object {c++ debug}] != ""} {
> + untested completion.exp
> + return -1
> +}
> +
> +if {[gdb_compile "${testfile}.o" ${binfile} executable {c++ debug}] != "" } {
> + untested completion.exp
> + return -1
> +}
s/completions.exp/cpcompletion.exp/
> +
> +send_gdb "p foo1.g\t"
> +gdb_expect {
> + -re "^p foo1\\.get_foo $"\
> + { send_gdb "()\n"
> + gdb_expect {
> + -re "^.* = 0.*$gdb_prompt $"\
> + { pass "complete 'p foo1.g'"}
> + -re ".*$gdb_prompt $" { fail "complete 'p foo1.g'"}
> + timeout {fail "(timeout) complete 'p foo1.g'"}
> + }
> + }
> + -re ".*$gdb_prompt $" { fail "complete 'p foo1.g'" }
> + timeout { fail "(timeout) complete 'p foo1.g' 2" }
> + }
> +
(I wish we had a function we could call that abstracted and made easier to
write/read these completion tests.)
What do you think about extending the test a little bit?
I think it would be nice to have,
Plain inheritance testing that the base class methods are completed.
--
Inheritance + masking, something like:
class FooBase
{
public:
int get_foo () { return 1; }
}
class Foo : public FooBase
{
public:
int get_foo () { ... }
}
Foo foo1;
From my testing, foo1.g<tab> will make get_foo only show up once,
which is fine, IMO.
A possible *future* improvement would be go show:
foo1.<tab>
foo1.FooBase::get_foo
foo1.get_foo
(foo1.Foo::get_foo too perhaps, not sure)
--
Masking, changing type:
class FooBase
{
private:
int get_foo;
}
class Foo : public FooBase
{
public:
int get_foo () { ... }
}
Foo foo1;
This case, foo1.g<tab> completes to foo1.get_foo, but 'p foo1.get_foo'
prints FooBase::get_foo. It's a bit confusing, especially since
get_foo is private in the base class --- not related to your patch,
just pointing it out. This is somewhat related to the future improvement I
mentioned above. That is, maybe showing:
foo1.<tab>
foo1.FooBase::get_foo
foo1.get_foo
Or:
foo1.g<tab>
foo1.get_foo
foo1.get_foo(
would make things clearer.
--
Anonymous struct with method,
struct
{
int get_foo () { ... }
} a;
a.<tab>
get_foo
--
Also, would it make sense to add a test that made sure the
ctors aren't completed?
--
Consider any test additions you make pre-approved.
--
Pedro Alves
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2008-10-21 22:25 ` Pedro Alves
@ 2009-02-03 1:02 ` Tom Tromey
2009-02-03 1:10 ` Tom Tromey
2009-02-03 1:20 ` Pedro Alves
0 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2009-02-03 1:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
Finally getting back to this old patch...
Tom> This patch fixes PR gdb/2489.
Tom> The bug is that field name completion does not consider methods.
Tom> + for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
Pedro> ^ missing space
Fixed.
Pedro> + for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
Pedro> + {
Pedro> + char *name = TYPE_FN_FIELDLIST_NAME (type, i);
Pedro> + if (name && ! strncmp (name, fieldname, namelen))
Pedro> + {
Pedro> + if (!type_name)
Pedro> + type_name = type_name_no_tag (type);
Pedro> + /* Omit constructors from the completion list. */
Pedro> + if (strcmp (type_name, name))
Pedro> + {
Pedro> Can type_name ever be NULL here then?
The reason for the check here is that we compute type_name the first
time we need it. It is initialized to NULL, but only set once.
Tom> +# Please email any bugs, comments, and/or additions to this file to:
Tom> +# bug-gdb@prep.ai.mit.edu
Pedro> Please remove this bit.
Whoops, fixed.
Tom> + untested completion.exp
Pedro> s/completions.exp/cpcompletion.exp/
Thanks -- fixed.
Pedro> (I wish we had a function we could call that abstracted and
Pedro> made easier to write/read these completion tests.)
I rewrote the tests to use the "complete" command rather than sending
a TAB. This makes them much simpler.
Pedro> What do you think about extending the test a little bit?
Pedro> I think it would be nice to have,
Pedro> Plain inheritance testing that the base class methods are completed.
[...]
Pedro> Inheritance + masking, something like:
[...]
Pedro> Anonymous struct with method,
[...]
Pedro> Also, would it make sense to add a test that made sure the
Pedro> ctors aren't completed?
I added these, thanks for the suggestions.
I've appended the final patch. I am checking this in.
Tom
2009-02-02 Tom Tromey <tromey@redhat.com>
PR gdb/2489:
* completer.c (count_struct_fields): Count method names.
(add_struct_fields): Add matching method names.
2009-02-02 Tom Tromey <tromey@redhat.com>
* gdb.cp/Makefile.in (EXECUTABLES): Add pr2489.
* gdb.cp/pr2489.cc: New file.
* gdb.cp/cpcompletion.exp: New file.
Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.30
diff -u -r1.30 completer.c
--- completer.c 3 Jan 2009 05:57:51 -0000 1.30
+++ completer.c 3 Feb 2009 00:56:41 -0000
@@ -339,7 +339,7 @@
}
/* Helper for expression_completer which recursively counts the number
- of named fields in a structure or union type. */
+ of named fields and methods in a structure or union type. */
static int
count_struct_fields (struct type *type)
{
@@ -353,17 +353,25 @@
else if (TYPE_FIELD_NAME (type, i))
++result;
}
+
+ for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
+ {
+ if (TYPE_FN_FIELDLIST_NAME (type, i))
+ ++result;
+ }
+
return result;
}
-/* Helper for expression_completer which recursively adds field names
- from TYPE, a struct or union type, to the array OUTPUT. This
- function assumes that OUTPUT is correctly-sized. */
+/* Helper for expression_completer which recursively adds field and
+ method names from TYPE, a struct or union type, to the array
+ OUTPUT. This function assumes that OUTPUT is correctly-sized. */
static void
add_struct_fields (struct type *type, int *nextp, char **output,
char *fieldname, int namelen)
{
int i;
+ char *type_name = NULL;
CHECK_TYPEDEF (type);
for (i = 0; i < TYPE_NFIELDS (type); ++i)
@@ -378,6 +386,22 @@
++*nextp;
}
}
+
+ for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
+ {
+ char *name = TYPE_FN_FIELDLIST_NAME (type, i);
+ if (name && ! strncmp (name, fieldname, namelen))
+ {
+ if (!type_name)
+ type_name = type_name_no_tag (type);
+ /* Omit constructors from the completion list. */
+ if (strcmp (type_name, name))
+ {
+ output[*nextp] = xstrdup (name);
+ ++*nextp;
+ }
+ }
+ }
}
/* Complete on expressions. Often this means completing on symbol
Index: testsuite/gdb.cp/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/Makefile.in,v
retrieving revision 1.3
diff -u -r1.3 Makefile.in
--- testsuite/gdb.cp/Makefile.in 28 Mar 2007 00:32:41 -0000 1.3
+++ testsuite/gdb.cp/Makefile.in 3 Feb 2009 00:56:43 -0000
@@ -4,7 +4,7 @@
EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
derivation inherit local member-ptr method misc \
overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
- ref-types ref-params method2
+ ref-types ref-params method2 pr2489
all info install-info dvi install uninstall installcheck check:
@echo "Nothing to be done for $@..."
Index: testsuite/gdb.cp/cpcompletion.exp
===================================================================
RCS file: testsuite/gdb.cp/cpcompletion.exp
diff -N testsuite/gdb.cp/cpcompletion.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/cpcompletion.exp 3 Feb 2009 00:56:43 -0000
@@ -0,0 +1,72 @@
+# Copyright 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile pr2489
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile "${srcdir}/${subdir}/${testfile}.cc" "${testfile}.o" object {c++ debug}] != ""} {
+ untested cpcompletion.exp
+ return -1
+}
+
+if {[gdb_compile "${testfile}.o" ${binfile} executable {c++ debug}] != "" } {
+ untested cpcompletion.exp
+ return -1
+}
+
+gdb_exit
+
+# Don't let a .inputrc file or an existing setting of INPUTRC mess up
+# the test results. Even if /dev/null doesn't exist on the particular
+# platform, the readline library will use the default setting just by
+# failing to open the file. OTOH, opening /dev/null successfully will
+# also result in the default settings being used since nothing will be
+# read from this file.
+global env
+if [info exists env(INPUTRC)] {
+ set old_inputrc $env(INPUTRC)
+}
+set env(INPUTRC) "/dev/null"
+
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "Set breakpoint here" ${testfile}.cc]
+
+if {![runto $bp_location]} {
+ perror "test suppressed"
+}
+
+# This also tests inheritance -- completion should only see a single
+# "get_foo".
+gdb_test "complete p foo1.g" "p foo1\\.get_foo"
+
+# Test inheritance without overriding.
+gdb_test "complete p foo1.base" "p foo1\\.base_function_only"
+
+# Test non-completion of constructor names.
+gdb_test "complete p foo1.Fo" "p foo1\\.Foofoo"
+
+# Test completion with an anonymous struct.
+gdb_test "complete p a.g" "p a\\.get"
Index: testsuite/gdb.cp/pr2489.cc
===================================================================
RCS file: testsuite/gdb.cp/pr2489.cc
diff -N testsuite/gdb.cp/pr2489.cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr2489.cc 3 Feb 2009 00:56:43 -0000
@@ -0,0 +1,51 @@
+
+class Base
+{
+public:
+ virtual int get_foo () { return 1; }
+ int base_function_only () { return 2; }
+};
+
+class Foo : public Base
+{
+
+private:
+ int foo_value;
+
+public:
+ Foo () { foo_value = 0;}
+ Foo (int i) { foo_value = i;}
+ ~Foo () { }
+ void set_foo (int value);
+ int get_foo ();
+
+ // Something similar to a constructor name.
+ void Foofoo ();
+
+ bool operator== (const Foo &other) { return foo_value == other.foo_value; }
+};
+
+void Foo::set_foo (int value)
+{
+ foo_value = value;
+}
+
+int Foo::get_foo ()
+{
+ return foo_value;
+}
+
+void Foo::Foofoo ()
+{
+}
+
+int main ()
+{
+ // Anonymous struct with method.
+ struct {
+ int get() { return 5; }
+ } a;
+ Foo foo1;
+ foo1.set_foo (42); // Set breakpoint here.
+ return 0;
+}
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2009-02-03 1:02 ` Tom Tromey
@ 2009-02-03 1:10 ` Tom Tromey
2009-02-03 1:20 ` Pedro Alves
1 sibling, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2009-02-03 1:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
Tom> 2009-02-02 Tom Tromey <tromey@redhat.com>
Tom> PR gdb/2489:
Tom> * completer.c (count_struct_fields): Count method names.
Tom> (add_struct_fields): Add matching method names.
Whoops! After committing, I remembered that this patch predates the
Bugzilla move.
I am checking in this patch to rename the test case. I'm also going
to fix gdb/ChangeLog to point to the correct bug.
Tom
2009-02-02 Tom Tromey <tromey@redhat.com>
* gdb.cp/cpcompletion.exp: Name the test "pr9594".
* gdb.cp/pr2489.cc: Rename...
* gdb.cp/pr9594.cc: ... to this.
Index: testsuite/gdb.cp/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/Makefile.in,v
retrieving revision 1.4
diff -u -r1.4 Makefile.in
--- testsuite/gdb.cp/Makefile.in 3 Feb 2009 01:00:40 -0000 1.4
+++ testsuite/gdb.cp/Makefile.in 3 Feb 2009 01:06:12 -0000
@@ -4,7 +4,7 @@
EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
derivation inherit local member-ptr method misc \
overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
- ref-types ref-params method2 pr2489
+ ref-types ref-params method2 pr9594
all info install-info dvi install uninstall installcheck check:
@echo "Nothing to be done for $@..."
Index: testsuite/gdb.cp/cpcompletion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/cpcompletion.exp,v
retrieving revision 1.1
diff -u -r1.1 cpcompletion.exp
--- testsuite/gdb.cp/cpcompletion.exp 3 Feb 2009 01:00:40 -0000 1.1
+++ testsuite/gdb.cp/cpcompletion.exp 3 Feb 2009 01:06:13 -0000
@@ -21,7 +21,7 @@
if { [skip_cplus_tests] } { continue }
-set testfile pr2489
+set testfile pr9594
set binfile ${objdir}/${subdir}/${testfile}
if {[gdb_compile "${srcdir}/${subdir}/${testfile}.cc" "${testfile}.o" object {c++ debug}] != ""} {
Index: testsuite/gdb.cp/pr2489.cc
===================================================================
RCS file: testsuite/gdb.cp/pr2489.cc
diff -N testsuite/gdb.cp/pr2489.cc
--- testsuite/gdb.cp/pr2489.cc 3 Feb 2009 01:00:40 -0000 1.1
+++ /dev/null 1 Jan 1970 00:00:00 -0000
@@ -1,51 +0,0 @@
-
-class Base
-{
-public:
- virtual int get_foo () { return 1; }
- int base_function_only () { return 2; }
-};
-
-class Foo : public Base
-{
-
-private:
- int foo_value;
-
-public:
- Foo () { foo_value = 0;}
- Foo (int i) { foo_value = i;}
- ~Foo () { }
- void set_foo (int value);
- int get_foo ();
-
- // Something similar to a constructor name.
- void Foofoo ();
-
- bool operator== (const Foo &other) { return foo_value == other.foo_value; }
-};
-
-void Foo::set_foo (int value)
-{
- foo_value = value;
-}
-
-int Foo::get_foo ()
-{
- return foo_value;
-}
-
-void Foo::Foofoo ()
-{
-}
-
-int main ()
-{
- // Anonymous struct with method.
- struct {
- int get() { return 5; }
- } a;
- Foo foo1;
- foo1.set_foo (42); // Set breakpoint here.
- return 0;
-}
Index: testsuite/gdb.cp/pr9594.cc
===================================================================
RCS file: testsuite/gdb.cp/pr9594.cc
diff -N testsuite/gdb.cp/pr9594.cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr9594.cc 3 Feb 2009 01:06:13 -0000
@@ -0,0 +1,51 @@
+
+class Base
+{
+public:
+ virtual int get_foo () { return 1; }
+ int base_function_only () { return 2; }
+};
+
+class Foo : public Base
+{
+
+private:
+ int foo_value;
+
+public:
+ Foo () { foo_value = 0;}
+ Foo (int i) { foo_value = i;}
+ ~Foo () { }
+ void set_foo (int value);
+ int get_foo ();
+
+ // Something similar to a constructor name.
+ void Foofoo ();
+
+ bool operator== (const Foo &other) { return foo_value == other.foo_value; }
+};
+
+void Foo::set_foo (int value)
+{
+ foo_value = value;
+}
+
+int Foo::get_foo ()
+{
+ return foo_value;
+}
+
+void Foo::Foofoo ()
+{
+}
+
+int main ()
+{
+ // Anonymous struct with method.
+ struct {
+ int get() { return 5; }
+ } a;
+ Foo foo1;
+ foo1.set_foo (42); // Set breakpoint here.
+ return 0;
+}
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2009-02-03 1:02 ` Tom Tromey
2009-02-03 1:10 ` Tom Tromey
@ 2009-02-03 1:20 ` Pedro Alves
2009-02-03 1:38 ` Tom Tromey
1 sibling, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2009-02-03 1:20 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Tuesday 03 February 2009 00:59:46, Tom Tromey wrote:
> Pedro> + for (i = TYPE_NFN_FIELDS (type) - 1; i >=0; --i)
> Pedro> + {
> Pedro> + char *name = TYPE_FN_FIELDLIST_NAME (type, i);
> Pedro> + if (name && ! strncmp (name, fieldname, namelen))
> Pedro> + {
> Pedro> + if (!type_name)
> Pedro> + type_name = type_name_no_tag (type);
> Pedro> + /* Omit constructors from the completion list. */
> Pedro> + if (strcmp (type_name, name))
> Pedro> + {
>
> Pedro> Can type_name ever be NULL here then?
>
> The reason for the check here is that we compute type_name the first
> time we need it. It is initialized to NULL, but only set once.
Oh, sorry, I guess I wasn't clear. I meant after the
type_name_no_tag call, at the strcmp line.
I mainly asked due to this:
/* Return a typename for a struct/union/enum type without "struct ",
"union ", or "enum ". If the type has a NULL name, return NULL. */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
char *
type_name_no_tag (const struct type *type)
{
if (TYPE_TAG_NAME (type) != NULL)
return TYPE_TAG_NAME (type);
/* Is there code which expects this to return the name if there is
no tag name? My guess is that this is mainly used for C++ in
cases where the two will always be the same. */
return TYPE_NAME (type);
}
I don't know if that can happen here. That was also the reason I
suggested an annonymous struct/class test.
On Tuesday 03 February 2009 00:59:46, Tom Tromey wrote:
> Pedro> (I wish we had a function we could call that abstracted and
> Pedro> made easier to write/read these completion tests.)
>
> I rewrote the tests to use the "complete" command rather than sending
> a TAB. This makes them much simpler.
>
Indeed! Much nicer.
--
Pedro Alves
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2009-02-03 1:20 ` Pedro Alves
@ 2009-02-03 1:38 ` Tom Tromey
2009-02-03 19:20 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2009-02-03 1:38 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
Pedro> Oh, sorry, I guess I wasn't clear. I meant after the
Pedro> type_name_no_tag call, at the strcmp line.
Oh, you were plenty clear -- I just misunderstood.
Pedro> I mainly asked due to this:
Pedro> /* Return a typename for a struct/union/enum type without "struct ",
Pedro> "union ", or "enum ". If the type has a NULL name, return NULL. */
Pedro> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Pedro> char *
Pedro> type_name_no_tag (const struct type *type)
[...]
Pedro> I don't know if that can happen here. That was also the reason I
Pedro> suggested an annonymous struct/class test.
I looked at the anonymous test struct case and I see:
(top) p type_name
$2 = 0x8fa3f1e "<anonymous struct>"
However, this appears to come from g++:
<2><361>: Abbrev Number: 33 (DW_TAG_structure_type)
<362> DW_AT_name : (indirect string, offset: 0x172): <anonymous struct>
I seem to recall that this is actually a change in g++.
I tried with an older g++ and the dwarf says:
<2><58b>: Abbrev Number: 29 (DW_TAG_structure_type)
DW_AT_sibling : <5ab>
DW_AT_name : ._0
... which just seems weird.
Anyway, I suspect that you are correct, and we could see a NULL here,
but I don't know how to make it happen. I will add a check for NULL
here tomorrow.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: RFA: fix PR gdb/2489
2009-02-03 1:38 ` Tom Tromey
@ 2009-02-03 19:20 ` Tom Tromey
0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2009-02-03 19:20 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
Tom> Anyway, I suspect that you are correct, and we could see a NULL here,
Tom> but I don't know how to make it happen. I will add a check for NULL
Tom> here tomorrow.
I am checking in the appended to address this potential problem.
Built and regtested on x86-64 (compile farm).
thanks,
Tom
2009-02-03 Tom Tromey <tromey@redhat.com>
* completer.c (add_struct_fields): Check type_name against NULL
before use.
diff --git a/gdb/completer.c b/gdb/completer.c
index 5d7225f..298cdd0 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -371,6 +371,7 @@ add_struct_fields (struct type *type, int *nextp, char **output,
char *fieldname, int namelen)
{
int i;
+ int computed_type_name = 0;
char *type_name = NULL;
CHECK_TYPEDEF (type);
@@ -392,10 +393,13 @@ add_struct_fields (struct type *type, int *nextp, char **output,
char *name = TYPE_FN_FIELDLIST_NAME (type, i);
if (name && ! strncmp (name, fieldname, namelen))
{
- if (!type_name)
- type_name = type_name_no_tag (type);
+ if (!computed_type_name)
+ {
+ type_name = type_name_no_tag (type);
+ computed_type_name = 1;
+ }
/* Omit constructors from the completion list. */
- if (strcmp (type_name, name))
+ if (type_name && strcmp (type_name, name))
{
output[*nextp] = xstrdup (name);
++*nextp;
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-02-03 19:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-01 19:15 RFA: fix PR gdb/2489 Tom Tromey
2008-10-21 22:25 ` Pedro Alves
2009-02-03 1:02 ` Tom Tromey
2009-02-03 1:10 ` Tom Tromey
2009-02-03 1:20 ` Pedro Alves
2009-02-03 1:38 ` Tom Tromey
2009-02-03 19:20 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox