[1/1] gdb: Use a typedef's scoped type name to identify local typefs

Message ID 20211125111604.311022-1-christina.schimpe@intel.com
State Superseded
Headers show
Series
  • [1/1] gdb: Use a typedef's scoped type name to identify local typefs
Related show

Commit Message

Mike Frysinger via Gdb-patches Nov. 25, 2021, 11:16 a.m.
GDB prints the wrong type for typedefs in case there is another typedef
available for the same raw type (gdb/16040).  The reason is that the
current hashmap based substitution mechanism always compares the target
type of a typedef and not its scoped name.

The original output of GDB for a program like

~~~~
namespace ns
{
  typedef double scoped_double;
}

typedef double global_double;

class TypedefHolder
{
public:
  double a;
  ns::scoped_double b;
  global_double c;

private:
  typedef double class_double;
  class_double d;

  double method1(ns::scoped_double) { return 24.0; }
  double method2(global_double) { return 24.0; }
};

int main()
{
  TypedefHolder th;
  return 0;
}
~~~~

is
~~~~

(gdb) b 27
Breakpoint 1 at 0x1131: file TypedefHolder.cc, line 27.
(gdb) r
Starting program: /tmp/typedefholder

Breakpoint 1, main () at TypedefHolder.cc:27
27	  return 0;
(gdb) ptype th
type = class TypedefHolder {
  public:
    class_double a;
    class_double b;
    class_double c;
  private:
    class_double d;

    class_double method1(class_double);
    class_double method2(class_double);

    typedef double class_double;
}
~~~~

Basically all attributes of a class which have the raw type "double" are
substituted by "class_double".

With the patch the output is the following

~~~~
type = class TypedefHolder {
  public:
    double a;
    ns::scoped_double b;
    global_double c;
  private:
    class_double d;

    double method1(ns::scoped_double);
    double method2(global_double);

    typedef double class_double;
}
~~~~

Any feedback about this patch is appreciated.

gdb/ChangeLog:
2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>
	* gdb/typeprint.c: Change typedef comparison value.

gdb/testsuite/ChangeLog:
2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>
	* gdb.cp/ptype-flags.cc: New test class.
	* gdb.cp/ptype-flags.exp: New test.
---
 gdb/testsuite/gdb.cp/ptype-flags.cc  | 23 ++++++++
 gdb/testsuite/gdb.cp/ptype-flags.exp | 88 ++++++++++++++++++++++------
 gdb/typeprint.c                      |  6 +-
 3 files changed, 97 insertions(+), 20 deletions(-)

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Comments

Mike Frysinger via Gdb-patches Nov. 25, 2021, 2:12 p.m. | #1
Thanks for looking at this. I only have one question, and a minor comment

On 11/25/21 08:16, Christina Schimpe via Gdb-patches wrote:
> GDB prints the wrong type for typedefs in case there is another typedef

> available for the same raw type (gdb/16040).  The reason is that the

> current hashmap based substitution mechanism always compares the target

> type of a typedef and not its scoped name.

> 

> The original output of GDB for a program like

> 

> ~~~~

> namespace ns

> {

>    typedef double scoped_double;

> }

> 

> typedef double global_double;

> 

> class TypedefHolder

> {

> public:

>    double a;

>    ns::scoped_double b;

>    global_double c;

> 

> private:

>    typedef double class_double;

>    class_double d;

> 

>    double method1(ns::scoped_double) { return 24.0; }

>    double method2(global_double) { return 24.0; }

> };

> 

> int main()

> {

>    TypedefHolder th;

>    return 0;

> }

> ~~~~

> 

> is

> ~~~~

> 

> (gdb) b 27

> Breakpoint 1 at 0x1131: file TypedefHolder.cc, line 27.

> (gdb) r

> Starting program: /tmp/typedefholder

> 

> Breakpoint 1, main () at TypedefHolder.cc:27

> 27	  return 0;

> (gdb) ptype th

> type = class TypedefHolder {

>    public:

>      class_double a;

>      class_double b;

>      class_double c;

>    private:

>      class_double d;

> 

>      class_double method1(class_double);

>      class_double method2(class_double);

> 

>      typedef double class_double;

> }

> ~~~~

> 

> Basically all attributes of a class which have the raw type "double" are

> substituted by "class_double".

> 

> With the patch the output is the following

> 

> ~~~~

> type = class TypedefHolder {

>    public:

>      double a;

>      ns::scoped_double b;

>      global_double c;

>    private:

>      class_double d;

> 

>      double method1(ns::scoped_double);

>      double method2(global_double);

> 

>      typedef double class_double;

> }

> ~~~~

> 

> Any feedback about this patch is appreciated.

> 

> gdb/ChangeLog:

> 2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>

> 	* gdb/typeprint.c: Change typedef comparison value.

> 

> gdb/testsuite/ChangeLog:

> 2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>

> 	* gdb.cp/ptype-flags.cc: New test class.

> 	* gdb.cp/ptype-flags.exp: New test.

We don't require changelogs anymore, only if the patch is going to gdb-11-branch.

> ---

>   gdb/testsuite/gdb.cp/ptype-flags.cc  | 23 ++++++++

>   gdb/testsuite/gdb.cp/ptype-flags.exp | 88 ++++++++++++++++++++++------

>   gdb/typeprint.c                      |  6 +-

>   3 files changed, 97 insertions(+), 20 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.cc b/gdb/testsuite/gdb.cp/ptype-flags.cc

> index fc92d3950c..564d272e57 100644

> --- a/gdb/testsuite/gdb.cp/ptype-flags.cc

> +++ b/gdb/testsuite/gdb.cp/ptype-flags.cc

> @@ -38,7 +38,30 @@ public:

>     double method(void) { return 23.0; }

>   };

>   

> +namespace ns

> +{

> +  typedef double scoped_double;

> +}

> +

> +typedef double global_double;

> +

> +class TypedefHolder

> +{

> +public:

> +  double a;

> +  ns::scoped_double b;

> +  global_double c;

> +

> +private:

> +  typedef double class_double;

> +  class_double d;

> +

> +  double method1(ns::scoped_double) { return 24.0; }

> +  double method2(global_double) { return 24.0; }

> +};

> +

>   Holder<int> value;

> +TypedefHolder value2;

>   

>   int main()

>   {

> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp

> index c368415793..c41e8f8744 100644

> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp

> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp

> @@ -33,7 +33,9 @@ if ![runto_main] then {

>   gdb_test_no_output "set language c++" ""

>   gdb_test_no_output "set width 0" ""

>   

> -proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

> +proc do_check_holder \

> +    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

> +

>       set contents {

>   	{ base "public Base<T>" }

>   	{ field public "Simple<T> t;" }

> @@ -62,24 +64,76 @@ proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

>   	"" {} $flags

>   }

>   

> -do_check "basic test"

> -do_check "no methods" "/m" 1 0

> -do_check "no typedefs" "/t" 0 1

> -do_check "no methods or typedefs" "/mt" 0 0

> +proc do_check_typedef_holder \

> +    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

> +

> +    set contents {

> +	{ field public "double a;" }

> +	{ field public "ns::scoped_double b;" }

> +	{ field public "global_double c;" }

> +    }

> +

> +    if {$show_typedefs} {

> +	lappend contents { typedef private "typedef double class_double;" }

> +    }

> +

> +    if {$show_methods} {

> +	lappend contents { method private "double method1(ns::scoped_double);" }

> +	lappend contents { method private "double method2(global_double);" }

> +    }

> +

> +    if {$raw} {

> +	lappend contents { field private "TypedefHolder::class_double d;" }

> +    } else {

> +	lappend contents { field private "class_double d;" }

> +    }

> +

> +    cp_test_ptype_class value2 $name "class" "TypedefHolder" $contents \

> +	"" {} $flags

> +}

>   

> -do_check "raw" "/r" 1 1 1

> -do_check "raw no methods" "/rm" 1 0 1

> -do_check "raw no typedefs" "/rt" 0 1 1

> -do_check "raw no methods or typedefs" "/rmt" 0 0 1

> +do_check_holder "basic test"

> +do_check_holder "no methods" "/m" 1 0

> +do_check_holder "no typedefs" "/t" 0 1

> +do_check_holder "no methods or typedefs" "/mt" 0 0

> +do_check_typedef_holder "typdefs class: basic test"

> +do_check_typedef_holder "typdefs class: no methods" "/m" 1 0

> +do_check_typedef_holder "typdefs class: no typedefs" "/t" 0 1 0

> +do_check_typedef_holder "typdefs class:no methods or typedefs" "/mt" 0 0

> +

> +do_check_holder "raw" "/r" 1 1 1

> +do_check_holder "raw no methods" "/rm" 1 0 1

> +do_check_holder "raw no typedefs" "/rt" 0 1 1

> +do_check_holder "raw no methods or typedefs" "/rmt" 0 0 1

> +do_check_typedef_holder "typedef class: raw" "/r" 1 1 1

> +do_check_typedef_holder "typedef class: raw no methods" "/rm" 1 0 1

> +do_check_typedef_holder "typedef class: raw no typedefs" "/rt" 0 1 1

> +do_check_typedef_holder "typedef class: raw no methods or typedefs" "/rmt" 0 0 1

>   

>   gdb_test_no_output "set print type methods off"

> -do_check "basic test, default methods off" "" 1 0

> -do_check "methods, default methods off" "/M" 1 1

> -do_check "no typedefs, default methods off" "/t" 0 0

> -do_check "methods, no typedefs, default methods off" "/Mt" 0 1

> +do_check_holder "basic test, default methods off" "" 1 0

> +do_check_holder "methods, default methods off" "/M" 1 1

> +do_check_holder "no typedefs, default methods off" "/t" 0 0

> +do_check_holder "methods, no typedefs, default methods off" "/Mt" 0 1

> +do_check_typedef_holder \

> +    "typedef class: basic test, default methods off" "" 1 0

> +do_check_typedef_holder \

> +    "typedef class: methods, default methods off" "/M" 1 1

> +do_check_typedef_holder \

> +    "typedef class: no typedefs, default methods off" "/t" 0 0

> +do_check_typedef_holder \

> +    "typedef class: methods, no typedefs, default methods off" "/Mt" 0 1

>   

>   gdb_test_no_output "set print type typedefs off"

> -do_check "basic test, default methods+typedefs off" "" 0 0

> -do_check "methods, default methods+typedefs off" "/M" 0 1

> -do_check "typedefs, default methods+typedefs off" "/T" 1 0

> -do_check "methods typedefs, default methods+typedefs off" "/MT" 1 1

> +do_check_holder "basic test, default methods+typedefs off" "" 0 0

> +do_check_holder "methods, default methods+typedefs off" "/M" 0 1

> +do_check_holder "typedefs, default methods+typedefs off" "/T" 1 0

> +do_check_holder "methods typedefs, default methods+typedefs off" "/MT" 1 1

> +do_check_typedef_holder \

> +    "typedef class: basic test, default methods+typedefs off" "" 0 0

> +do_check_typedef_holder \

> +    "typedef class: methods, default methods+typedefs off" "/M" 0 1

> +do_check_typedef_holder \

> +    "typedef class: typedefs, default methods+typedefs off" "/T" 1 0

> +do_check_typedef_holder \

> +    "typedef class: methods typedefs, default methods+typedefs off" "/MT" 1 1

> diff --git a/gdb/typeprint.c b/gdb/typeprint.c

> index 1312111b60..f1b0ce0a71 100644

> --- a/gdb/typeprint.c

> +++ b/gdb/typeprint.c

> @@ -201,9 +201,8 @@ static hashval_t

>   hash_typedef_field (const void *p)

>   {

>     const struct decl_field *tf = (const struct decl_field *) p;

> -  struct type *t = check_typedef (tf->type);

>   

> -  return htab_hash_string (TYPE_SAFE_NAME (t));

> +  return htab_hash_string (TYPE_SAFE_NAME (tf->type));

>   }

>   

>   /* An equality function for a typedef field.  */

> @@ -214,7 +213,8 @@ eq_typedef_field (const void *a, const void *b)

>     const struct decl_field *tfa = (const struct decl_field *) a;

>     const struct decl_field *tfb = (const struct decl_field *) b;

>   

> -  return types_equal (tfa->type, tfb->type);

> +  return (tfa->type->name () && tfb->type->name ()

> +    && (strcmp (tfa->type->name (), tfb->type->name ()) == 0));

>   }


Why did you change from types_equal to explicitly comparing names? Types_equal already makes this comparison, and if I put it back in the code, your testcase still passes. Am I mising something?

>   

>   /* See typeprint.h.  */

> 



-- 
Cheers!
Bruno Larsen
Mike Frysinger via Gdb-patches Nov. 26, 2021, 3:20 p.m. | #2
Hi Bruno, 

Thanks for the review. 
I investigated again the change from types_equal to explicitly comparing names and agree.
The change from types_equal to explicitly comparing names is not required. I missed that.
I will send a v2 soon.

Best,
Christina

-----Original Message-----
From: Bruno Larsen <blarsen@redhat.com> 

Sent: Thursday, November 25, 2021 3:13 PM
To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH 1/1] gdb: Use a typedef's scoped type name to identify local typefs


Thanks for looking at this. I only have one question, and a minor comment

On 11/25/21 08:16, Christina Schimpe via Gdb-patches wrote:
> GDB prints the wrong type for typedefs in case there is another 

> typedef available for the same raw type (gdb/16040).  The reason is 

> that the current hashmap based substitution mechanism always compares 

> the target type of a typedef and not its scoped name.

> 

> The original output of GDB for a program like

> 

> ~~~~

> namespace ns

> {

>    typedef double scoped_double;

> }

> 

> typedef double global_double;

> 

> class TypedefHolder

> {

> public:

>    double a;

>    ns::scoped_double b;

>    global_double c;

> 

> private:

>    typedef double class_double;

>    class_double d;

> 

>    double method1(ns::scoped_double) { return 24.0; }

>    double method2(global_double) { return 24.0; } };

> 

> int main()

> {

>    TypedefHolder th;

>    return 0;

> }

> ~~~~

> 

> is

> ~~~~

> 

> (gdb) b 27

> Breakpoint 1 at 0x1131: file TypedefHolder.cc, line 27.

> (gdb) r

> Starting program: /tmp/typedefholder

> 

> Breakpoint 1, main () at TypedefHolder.cc:27

> 27	  return 0;

> (gdb) ptype th

> type = class TypedefHolder {

>    public:

>      class_double a;

>      class_double b;

>      class_double c;

>    private:

>      class_double d;

> 

>      class_double method1(class_double);

>      class_double method2(class_double);

> 

>      typedef double class_double;

> }

> ~~~~

> 

> Basically all attributes of a class which have the raw type "double" 

> are substituted by "class_double".

> 

> With the patch the output is the following

> 

> ~~~~

> type = class TypedefHolder {

>    public:

>      double a;

>      ns::scoped_double b;

>      global_double c;

>    private:

>      class_double d;

> 

>      double method1(ns::scoped_double);

>      double method2(global_double);

> 

>      typedef double class_double;

> }

> ~~~~

> 

> Any feedback about this patch is appreciated.

> 

> gdb/ChangeLog:

> 2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>

> 	* gdb/typeprint.c: Change typedef comparison value.

> 

> gdb/testsuite/ChangeLog:

> 2021-11-25  Christina Schimpe  <christina.schimpe@intel.com>

> 	* gdb.cp/ptype-flags.cc: New test class.

> 	* gdb.cp/ptype-flags.exp: New test.

We don't require changelogs anymore, only if the patch is going to gdb-11-branch.

> ---

>   gdb/testsuite/gdb.cp/ptype-flags.cc  | 23 ++++++++

>   gdb/testsuite/gdb.cp/ptype-flags.exp | 88 ++++++++++++++++++++++------

>   gdb/typeprint.c                      |  6 +-

>   3 files changed, 97 insertions(+), 20 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.cc 

> b/gdb/testsuite/gdb.cp/ptype-flags.cc

> index fc92d3950c..564d272e57 100644

> --- a/gdb/testsuite/gdb.cp/ptype-flags.cc

> +++ b/gdb/testsuite/gdb.cp/ptype-flags.cc

> @@ -38,7 +38,30 @@ public:

>     double method(void) { return 23.0; }

>   };

>   

> +namespace ns

> +{

> +  typedef double scoped_double;

> +}

> +

> +typedef double global_double;

> +

> +class TypedefHolder

> +{

> +public:

> +  double a;

> +  ns::scoped_double b;

> +  global_double c;

> +

> +private:

> +  typedef double class_double;

> +  class_double d;

> +

> +  double method1(ns::scoped_double) { return 24.0; }

> +  double method2(global_double) { return 24.0; } };

> +

>   Holder<int> value;

> +TypedefHolder value2;

>   

>   int main()

>   {

> diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp 

> b/gdb/testsuite/gdb.cp/ptype-flags.exp

> index c368415793..c41e8f8744 100644

> --- a/gdb/testsuite/gdb.cp/ptype-flags.exp

> +++ b/gdb/testsuite/gdb.cp/ptype-flags.exp

> @@ -33,7 +33,9 @@ if ![runto_main] then {

>   gdb_test_no_output "set language c++" ""

>   gdb_test_no_output "set width 0" ""

>   

> -proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} 

> {raw 0}} {

> +proc do_check_holder \

> +    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

> +

>       set contents {

>   	{ base "public Base<T>" }

>   	{ field public "Simple<T> t;" }

> @@ -62,24 +64,76 @@ proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

>   	"" {} $flags

>   }

>   

> -do_check "basic test"

> -do_check "no methods" "/m" 1 0

> -do_check "no typedefs" "/t" 0 1

> -do_check "no methods or typedefs" "/mt" 0 0

> +proc do_check_typedef_holder \

> +    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {

> +

> +    set contents {

> +	{ field public "double a;" }

> +	{ field public "ns::scoped_double b;" }

> +	{ field public "global_double c;" }

> +    }

> +

> +    if {$show_typedefs} {

> +	lappend contents { typedef private "typedef double class_double;" }

> +    }

> +

> +    if {$show_methods} {

> +	lappend contents { method private "double method1(ns::scoped_double);" }

> +	lappend contents { method private "double method2(global_double);" }

> +    }

> +

> +    if {$raw} {

> +	lappend contents { field private "TypedefHolder::class_double d;" }

> +    } else {

> +	lappend contents { field private "class_double d;" }

> +    }

> +

> +    cp_test_ptype_class value2 $name "class" "TypedefHolder" $contents \

> +	"" {} $flags

> +}

>   

> -do_check "raw" "/r" 1 1 1

> -do_check "raw no methods" "/rm" 1 0 1 -do_check "raw no typedefs" 

> "/rt" 0 1 1 -do_check "raw no methods or typedefs" "/rmt" 0 0 1

> +do_check_holder "basic test"

> +do_check_holder "no methods" "/m" 1 0 do_check_holder "no typedefs" 

> +"/t" 0 1 do_check_holder "no methods or typedefs" "/mt" 0 0 

> +do_check_typedef_holder "typdefs class: basic test"

> +do_check_typedef_holder "typdefs class: no methods" "/m" 1 0 

> +do_check_typedef_holder "typdefs class: no typedefs" "/t" 0 1 0 

> +do_check_typedef_holder "typdefs class:no methods or typedefs" "/mt" 

> +0 0

> +

> +do_check_holder "raw" "/r" 1 1 1

> +do_check_holder "raw no methods" "/rm" 1 0 1 do_check_holder "raw no 

> +typedefs" "/rt" 0 1 1 do_check_holder "raw no methods or typedefs" 

> +"/rmt" 0 0 1 do_check_typedef_holder "typedef class: raw" "/r" 1 1 1 

> +do_check_typedef_holder "typedef class: raw no methods" "/rm" 1 0 1 

> +do_check_typedef_holder "typedef class: raw no typedefs" "/rt" 0 1 1 

> +do_check_typedef_holder "typedef class: raw no methods or typedefs" 

> +"/rmt" 0 0 1

>   

>   gdb_test_no_output "set print type methods off"

> -do_check "basic test, default methods off" "" 1 0 -do_check "methods, 

> default methods off" "/M" 1 1 -do_check "no typedefs, default methods 

> off" "/t" 0 0 -do_check "methods, no typedefs, default methods off" 

> "/Mt" 0 1

> +do_check_holder "basic test, default methods off" "" 1 0 

> +do_check_holder "methods, default methods off" "/M" 1 1 

> +do_check_holder "no typedefs, default methods off" "/t" 0 0 

> +do_check_holder "methods, no typedefs, default methods off" "/Mt" 0 1 

> +do_check_typedef_holder \

> +    "typedef class: basic test, default methods off" "" 1 0 

> +do_check_typedef_holder \

> +    "typedef class: methods, default methods off" "/M" 1 1 

> +do_check_typedef_holder \

> +    "typedef class: no typedefs, default methods off" "/t" 0 0 

> +do_check_typedef_holder \

> +    "typedef class: methods, no typedefs, default methods off" "/Mt" 

> +0 1

>   

>   gdb_test_no_output "set print type typedefs off"

> -do_check "basic test, default methods+typedefs off" "" 0 0 -do_check 

> "methods, default methods+typedefs off" "/M" 0 1 -do_check "typedefs, 

> default methods+typedefs off" "/T" 1 0 -do_check "methods typedefs, 

> default methods+typedefs off" "/MT" 1 1

> +do_check_holder "basic test, default methods+typedefs off" "" 0 0 

> +do_check_holder "methods, default methods+typedefs off" "/M" 0 1 

> +do_check_holder "typedefs, default methods+typedefs off" "/T" 1 0 

> +do_check_holder "methods typedefs, default methods+typedefs off" 

> +"/MT" 1 1 do_check_typedef_holder \

> +    "typedef class: basic test, default methods+typedefs off" "" 0 0 

> +do_check_typedef_holder \

> +    "typedef class: methods, default methods+typedefs off" "/M" 0 1 

> +do_check_typedef_holder \

> +    "typedef class: typedefs, default methods+typedefs off" "/T" 1 0 

> +do_check_typedef_holder \

> +    "typedef class: methods typedefs, default methods+typedefs off" 

> +"/MT" 1 1

> diff --git a/gdb/typeprint.c b/gdb/typeprint.c index 

> 1312111b60..f1b0ce0a71 100644

> --- a/gdb/typeprint.c

> +++ b/gdb/typeprint.c

> @@ -201,9 +201,8 @@ static hashval_t

>   hash_typedef_field (const void *p)

>   {

>     const struct decl_field *tf = (const struct decl_field *) p;

> -  struct type *t = check_typedef (tf->type);

>   

> -  return htab_hash_string (TYPE_SAFE_NAME (t));

> +  return htab_hash_string (TYPE_SAFE_NAME (tf->type));

>   }

>   

>   /* An equality function for a typedef field.  */ @@ -214,7 +213,8 @@ 

> eq_typedef_field (const void *a, const void *b)

>     const struct decl_field *tfa = (const struct decl_field *) a;

>     const struct decl_field *tfb = (const struct decl_field *) b;

>   

> -  return types_equal (tfa->type, tfb->type);

> +  return (tfa->type->name () && tfb->type->name ()

> +    && (strcmp (tfa->type->name (), tfb->type->name ()) == 0));

>   }


Why did you change from types_equal to explicitly comparing names? Types_equal already makes this comparison, and if I put it back in the code, your testcase still passes. Am I mising something?

>   

>   /* See typeprint.h.  */

> 



--
Cheers!
Bruno Larsen

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Patch

diff --git a/gdb/testsuite/gdb.cp/ptype-flags.cc b/gdb/testsuite/gdb.cp/ptype-flags.cc
index fc92d3950c..564d272e57 100644
--- a/gdb/testsuite/gdb.cp/ptype-flags.cc
+++ b/gdb/testsuite/gdb.cp/ptype-flags.cc
@@ -38,7 +38,30 @@  public:
   double method(void) { return 23.0; }
 };
 
+namespace ns
+{
+  typedef double scoped_double;
+}
+
+typedef double global_double;
+
+class TypedefHolder
+{
+public:
+  double a;
+  ns::scoped_double b;
+  global_double c;
+
+private:
+  typedef double class_double;
+  class_double d;
+
+  double method1(ns::scoped_double) { return 24.0; }
+  double method2(global_double) { return 24.0; }
+};
+
 Holder<int> value;
+TypedefHolder value2;
 
 int main()
 {
diff --git a/gdb/testsuite/gdb.cp/ptype-flags.exp b/gdb/testsuite/gdb.cp/ptype-flags.exp
index c368415793..c41e8f8744 100644
--- a/gdb/testsuite/gdb.cp/ptype-flags.exp
+++ b/gdb/testsuite/gdb.cp/ptype-flags.exp
@@ -33,7 +33,9 @@  if ![runto_main] then {
 gdb_test_no_output "set language c++" ""
 gdb_test_no_output "set width 0" ""
 
-proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {
+proc do_check_holder \
+    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {
+
     set contents {
 	{ base "public Base<T>" }
 	{ field public "Simple<T> t;" }
@@ -62,24 +64,76 @@  proc do_check {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {
 	"" {} $flags
 }
 
-do_check "basic test"
-do_check "no methods" "/m" 1 0
-do_check "no typedefs" "/t" 0 1
-do_check "no methods or typedefs" "/mt" 0 0
+proc do_check_typedef_holder \
+    {name {flags ""} {show_typedefs 1} {show_methods 1} {raw 0}} {
+
+    set contents {
+	{ field public "double a;" }
+	{ field public "ns::scoped_double b;" }
+	{ field public "global_double c;" }
+    }
+
+    if {$show_typedefs} {
+	lappend contents { typedef private "typedef double class_double;" }
+    }
+
+    if {$show_methods} {
+	lappend contents { method private "double method1(ns::scoped_double);" }
+	lappend contents { method private "double method2(global_double);" }
+    }
+
+    if {$raw} {
+	lappend contents { field private "TypedefHolder::class_double d;" }
+    } else {
+	lappend contents { field private "class_double d;" }
+    }
+
+    cp_test_ptype_class value2 $name "class" "TypedefHolder" $contents \
+	"" {} $flags
+}
 
-do_check "raw" "/r" 1 1 1
-do_check "raw no methods" "/rm" 1 0 1
-do_check "raw no typedefs" "/rt" 0 1 1
-do_check "raw no methods or typedefs" "/rmt" 0 0 1
+do_check_holder "basic test"
+do_check_holder "no methods" "/m" 1 0
+do_check_holder "no typedefs" "/t" 0 1
+do_check_holder "no methods or typedefs" "/mt" 0 0
+do_check_typedef_holder "typdefs class: basic test"
+do_check_typedef_holder "typdefs class: no methods" "/m" 1 0
+do_check_typedef_holder "typdefs class: no typedefs" "/t" 0 1 0
+do_check_typedef_holder "typdefs class:no methods or typedefs" "/mt" 0 0
+
+do_check_holder "raw" "/r" 1 1 1
+do_check_holder "raw no methods" "/rm" 1 0 1
+do_check_holder "raw no typedefs" "/rt" 0 1 1
+do_check_holder "raw no methods or typedefs" "/rmt" 0 0 1
+do_check_typedef_holder "typedef class: raw" "/r" 1 1 1
+do_check_typedef_holder "typedef class: raw no methods" "/rm" 1 0 1
+do_check_typedef_holder "typedef class: raw no typedefs" "/rt" 0 1 1
+do_check_typedef_holder "typedef class: raw no methods or typedefs" "/rmt" 0 0 1
 
 gdb_test_no_output "set print type methods off"
-do_check "basic test, default methods off" "" 1 0
-do_check "methods, default methods off" "/M" 1 1
-do_check "no typedefs, default methods off" "/t" 0 0
-do_check "methods, no typedefs, default methods off" "/Mt" 0 1
+do_check_holder "basic test, default methods off" "" 1 0
+do_check_holder "methods, default methods off" "/M" 1 1
+do_check_holder "no typedefs, default methods off" "/t" 0 0
+do_check_holder "methods, no typedefs, default methods off" "/Mt" 0 1
+do_check_typedef_holder \
+    "typedef class: basic test, default methods off" "" 1 0
+do_check_typedef_holder \
+    "typedef class: methods, default methods off" "/M" 1 1
+do_check_typedef_holder \
+    "typedef class: no typedefs, default methods off" "/t" 0 0
+do_check_typedef_holder \
+    "typedef class: methods, no typedefs, default methods off" "/Mt" 0 1
 
 gdb_test_no_output "set print type typedefs off"
-do_check "basic test, default methods+typedefs off" "" 0 0
-do_check "methods, default methods+typedefs off" "/M" 0 1
-do_check "typedefs, default methods+typedefs off" "/T" 1 0
-do_check "methods typedefs, default methods+typedefs off" "/MT" 1 1
+do_check_holder "basic test, default methods+typedefs off" "" 0 0
+do_check_holder "methods, default methods+typedefs off" "/M" 0 1
+do_check_holder "typedefs, default methods+typedefs off" "/T" 1 0
+do_check_holder "methods typedefs, default methods+typedefs off" "/MT" 1 1
+do_check_typedef_holder \
+    "typedef class: basic test, default methods+typedefs off" "" 0 0
+do_check_typedef_holder \
+    "typedef class: methods, default methods+typedefs off" "/M" 0 1
+do_check_typedef_holder \
+    "typedef class: typedefs, default methods+typedefs off" "/T" 1 0
+do_check_typedef_holder \
+    "typedef class: methods typedefs, default methods+typedefs off" "/MT" 1 1
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 1312111b60..f1b0ce0a71 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -201,9 +201,8 @@  static hashval_t
 hash_typedef_field (const void *p)
 {
   const struct decl_field *tf = (const struct decl_field *) p;
-  struct type *t = check_typedef (tf->type);
 
-  return htab_hash_string (TYPE_SAFE_NAME (t));
+  return htab_hash_string (TYPE_SAFE_NAME (tf->type));
 }
 
 /* An equality function for a typedef field.  */
@@ -214,7 +213,8 @@  eq_typedef_field (const void *a, const void *b)
   const struct decl_field *tfa = (const struct decl_field *) a;
   const struct decl_field *tfb = (const struct decl_field *) b;
 
-  return types_equal (tfa->type, tfb->type);
+  return (tfa->type->name () && tfb->type->name ()
+    && (strcmp (tfa->type->name (), tfb->type->name ()) == 0));
 }
 
 /* See typeprint.h.  */