Re: [rfc] get rid of redundant data in c++ and java

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

Daniel Jacobowitz wrote:
> On Wed, Feb 13, 2008 at 07:15:48PM -0500, Aleksandar Ristovski wrote:
>> Anyone has any comments on this?
> 
> Sorry, I'm very busy during the week and there does not seem to be
> another maintainer comfortable (or available) to review symbol table
> patches right now.  I'll get back to you as soon as I have the time;
> both for this and your other patch.
> 

Just curious if you had a chance to review? (this thread and this one:
http://sourceware.org/ml/gdb-patches/2008-02/msg00208.html)

Thanks,

Aleksandar Ristovski
QNX Software Systems

[rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

Hello,

Currently we synthesize typedef info for java and c++ (and ada, but I am not 
familiar with that language) for any symbol with STRUCT_DOMAIN domain. This can 
add up to a lot of redundant data. For partial symbols, we generate two 
identical names for the same symbol, once as struct second as typedef. The same 
happens with symbols, except we do not duplicate symbol name, but we do allocate 
another symbol struture.

Instead of generating redundant data for symbols with STRUCT_DOMAIN domain in 
java and c++ languages, treat STRUCT_DOMAIN as equal to VAR_DOMAIN for these two 
languages when looking up symbols.

See the patch for details.

Running tests on linux x86, no regressions.


---
Aleksandar Ristovski
QNX Software Systems



2008-02-08  Aleksandar Ristovski  <aristovski@qnx.com>

	* dwarf2read.c (add_partial_symbol): Do not add new psym for
	STRUCT_DOMAIN. Make sure you recognize c++ struct and java class
	as typedefs. See lookup_partial_symbol function.
	(new_symbol): Similar to add_partial_symbol, do not create
	symbol for the typedef. See lookup_block_symbol.
	* symtab.c (symbol_matches_domain): New function, takes care
	of dual meaning of STRUCT_DOMAIN symbol for c++ and java.
	(lookup_partial_symbol): Use symbol_matches_domain to see if the
	found psym domain matches the given domain.
	(lookup_block_symbol): Likewise.


[-- Attachment #2: duplicatesymbolfix.diff --]
[-- Type: text/plain, Size: 5925 bytes --]

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.251
diff -u -5 -p -p -r1.251 dwarf2read.c
--- gdb/dwarf2read.c	1 Feb 2008 22:45:13 -0000	1.251
+++ gdb/dwarf2read.c	9 Feb 2008 04:51:19 -0000
@@ -2006,15 +2006,14 @@ add_partial_symbol (struct partial_die_i
 			    || cu->language == language_java)
 			   ? &objfile->global_psymbols
 			   : &objfile->static_psymbols,
 			   0, (CORE_ADDR) 0, cu->language, objfile);
 
-      if (cu->language == language_cplus
-          || cu->language == language_java
-          || cu->language == language_ada)
+      if (cu->language == language_ada)
 	{
-	  /* For C++ and Java, these implicitly act as typedefs as well. */
+	  /* FIXME: Check if Ada really
+	     needs to implicitly set typedef.  */
 	  add_psymbol_to_list (actual_name, strlen (actual_name),
 			       VAR_DOMAIN, LOC_TYPEDEF,
 			       &objfile->global_psymbols,
 			       0, (CORE_ADDR) 0, cu->language, objfile);
 	}
@@ -7429,29 +7428,32 @@ new_symbol (struct die_info *die, struct
 			   && (cu->language == language_cplus
 			       || cu->language == language_java)
 			   ? &global_symbols : cu->list_in_scope);
 	  
 	    add_symbol_to_list (sym, list_to_add);
-
-	    /* The semantics of C++ state that "struct foo { ... }" also
-	       defines a typedef for "foo".  A Java class declaration also
-	       defines a typedef for the class.  Synthesize a typedef symbol
-	       so that "ptype foo" works as expected.  */
+	    
 	    if (cu->language == language_cplus
 		|| cu->language == language_java
 		|| cu->language == language_ada)
 	      {
-		struct symbol *typedef_sym = (struct symbol *)
-		  obstack_alloc (&objfile->objfile_obstack,
-				 sizeof (struct symbol));
-		*typedef_sym = *sym;
-		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		/* The symbol's name is already allocated along with
 		   this objfile, so we don't need to duplicate it for
 		   the type.  */
 		if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
 		  TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_SEARCH_NAME (sym);
+	      }
+ 
+	    /* FIXME: Check if ada really needs to synthesize the typedef
+	       or it can, as cplus and java, reuse STRUCT_DOMAIN in
+	       lookup_ functions.  */
+	    if (cu->language == language_ada)
+	      {
+		struct symbol *typedef_sym = (struct symbol *)
+		  obstack_alloc (&objfile->objfile_obstack,
+				 sizeof (struct symbol));
+		*typedef_sym = *sym;
+		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		add_symbol_to_list (typedef_sym, list_to_add);
 	      }
 	  }
 	  break;
 	case DW_TAG_typedef:
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.173
diff -u -5 -p -p -r1.173 symtab.c
--- gdb/symtab.c	5 Feb 2008 22:17:40 -0000	1.173
+++ gdb/symtab.c	9 Feb 2008 04:51:20 -0000
@@ -1577,10 +1577,28 @@ lookup_symbol_global (const char *name,
 
   return lookup_symbol_aux_psymtabs (GLOBAL_BLOCK, name, linkage_name,
 				     domain, symtab);
 }
 
+static int
+symbol_matches_domain (enum language symbol_language, 
+		      domain_enum symbol_domain,
+		      domain_enum domain)
+{
+  /* For c++ "struct foo { ... }" also defines a typedef for "foo".  
+     A Java class declaration also defines a typedef for the class.  */
+  if (symbol_language == language_cplus
+      || symbol_language  == language_java)
+    {
+      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN)
+	return 1;
+    }
+  /* For all other languages, strict match is required.  */
+  return (symbol_domain == domain);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    If LINKAGE_NAME is non-NULL, check in addition that the symbol's
    linkage name matches it.  Check the global symbols if GLOBAL, the
    static symbols if not */
 
@@ -1641,14 +1659,13 @@ lookup_partial_symbol (struct partial_sy
       while (top <= real_top
 	     && (linkage_name != NULL
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
 		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
-	  if (SYMBOL_DOMAIN (*top) == domain)
-	    {
-		  return (*top);
-	    }
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
+				     SYMBOL_DOMAIN (*top), domain))
+	    return (*top);
 	  top++;
 	}
     }
 
   /* Can't use a binary search or else we found during the binary search that
@@ -1656,11 +1673,12 @@ lookup_partial_symbol (struct partial_sy
 
   if (do_linear_search)
     {			
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (domain == SYMBOL_DOMAIN (*psym))
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym), 
+				     SYMBOL_DOMAIN (*psym), domain))
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
 		  : SYMBOL_MATCHES_SEARCH_NAME (*psym, name))
 		{
@@ -1841,11 +1859,12 @@ lookup_block_symbol (const struct block 
     {
       for (sym = dict_iter_name_first (BLOCK_DICT (block), name, &iter);
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    return sym;
 	}
       return NULL;
@@ -1862,11 +1881,12 @@ lookup_block_symbol (const struct block 
 
       for (sym = dict_iter_name_first (BLOCK_DICT (block), name, &iter);
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    {
 	      sym_found = sym;
 	      if (SYMBOL_CLASS (sym) != LOC_ARG &&

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

Aleksandar Ristovski wrote:
> Hello,
> 
> Currently we synthesize typedef info for java and c++ (and ada, but I am not 
> familiar with that language) for any symbol with STRUCT_DOMAIN domain. This can 
> add up to a lot of redundant data. 
...
> Instead of generating redundant data for symbols with STRUCT_DOMAIN domain in 
> java and c++ languages, treat STRUCT_DOMAIN as equal to VAR_DOMAIN for these two 
> languages when looking up symbols.
> 

Anyone has any comments on this?

Re: [rfc] get rid of redundant data in c++ and java

[Daniel Jacobowitz]

On Wed, Feb 13, 2008 at 07:15:48PM -0500, Aleksandar Ristovski wrote:
> Anyone has any comments on this?

Sorry, I'm very busy during the week and there does not seem to be
another maintainer comfortable (or available) to review symbol table
patches right now.  I'll get back to you as soon as I have the time;
both for this and your other patch.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] get rid of redundant data in c++ and java

[Daniel Jacobowitz]

On Sat, Feb 09, 2008 at 12:21:02AM -0500, Aleksandar Ristovski wrote:
> Hello,
>
> Currently we synthesize typedef info for java and c++ (and ada, but I am 
> not familiar with that language) for any symbol with STRUCT_DOMAIN domain. 
> This can add up to a lot of redundant data. For partial symbols, we 
> generate two identical names for the same symbol, once as struct second as 
> typedef. The same happens with symbols, except we do not duplicate symbol 
> name, but we do allocate another symbol struture.
>
> Instead of generating redundant data for symbols with STRUCT_DOMAIN domain 
> in java and c++ languages, treat STRUCT_DOMAIN as equal to VAR_DOMAIN for 
> these two languages when looking up symbols.

Sorry for taking so long to look at this.  It's a great idea and the
patch seems correct to me.  Just some cosmetic issues, and then it can
go in.

> -      if (cu->language == language_cplus
> -          || cu->language == language_java
> -          || cu->language == language_ada)
> +      if (cu->language == language_ada)
>  	{
> -	  /* For C++ and Java, these implicitly act as typedefs as well. */
> +	  /* FIXME: Check if Ada really
> +	     needs to implicitly set typedef.  */

If this works for C++ and Java, it will work for Ada too.  If you
can't test Ada, please post a final version of the patch and ask Joel
to run tests for you - I'm sure he won't mind getting all that memory
back for Ada too.

> -
> -	    /* The semantics of C++ state that "struct foo { ... }" also
> -	       defines a typedef for "foo".  A Java class declaration also
> -	       defines a typedef for the class.  Synthesize a typedef symbol
> -	       so that "ptype foo" works as expected.  */
> +	    

Since we still need to set the typedef name here, leave the first two
sentences of the comment.

> +  if (symbol_language == language_cplus
> +      || symbol_language  == language_java)

Only need one space there.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

Daniel Jacobowitz wrote:
> On Sat, Feb 09, 2008 at 12:21:02AM -0500, Aleksandar Ristovski wrote:
>> Hello,
>>
>> Currently we synthesize typedef info for java and c++ (and ada, but I am 
>> not familiar with that language) for any symbol with STRUCT_DOMAIN domain. 
>> This can add up to a lot of redundant data. For partial symbols, we 
>> generate two identical names for the same symbol, once as struct second as 
>> typedef. The same happens with symbols, except we do not duplicate symbol 
>> name, but we do allocate another symbol struture.
>>
>> Instead of generating redundant data for symbols with STRUCT_DOMAIN domain 
>> in java and c++ languages, treat STRUCT_DOMAIN as equal to VAR_DOMAIN for 
>> these two languages when looking up symbols.
> 
> Sorry for taking so long to look at this.  It's a great idea and the
> patch seems correct to me.  Just some cosmetic issues, and then it can
> go in.
> 
>> -      if (cu->language == language_cplus
>> -          || cu->language == language_java
>> -          || cu->language == language_ada)
>> +      if (cu->language == language_ada)
>>  	{
>> -	  /* For C++ and Java, these implicitly act as typedefs as well. */
>> +	  /* FIXME: Check if Ada really
>> +	     needs to implicitly set typedef.  */
> 
> If this works for C++ and Java, it will work for Ada too.  If you
> can't test Ada, please post a final version of the patch and ask Joel
> to run tests for you - I'm sure he won't mind getting all that memory
> back for Ada too.

Ok, here is the revised patch. Cc-ing Joel to take a look if it affects ada.

>> -
>> -	    /* The semantics of C++ state that "struct foo { ... }" also
>> -	       defines a typedef for "foo".  A Java class declaration also
>> -	       defines a typedef for the class.  Synthesize a typedef symbol
>> -	       so that "ptype foo" works as expected.  */
>> +	    
> 
> Since we still need to set the typedef name here, leave the first two
> sentences of the comment.

Sentences put back.

> 
>> +  if (symbol_language == language_cplus
>> +      || symbol_language  == language_java)
> 
> Only need one space there.
> 

One space.


Waiting for an OK to commit.


Thanks,

Aleksandar



ChangeLog:
	* dwarf2read.c (add_partial_symbol): Do not add new psym for
	STRUCT_DOMAIN. Make sure you recognize c++ struct and java and ada 
	class as typedefs. See lookup_partial_symbol function.
	(new_symbol): Similar to add_partial_symbol, do not create 
	symbol for the typedef. See lookup_block_symbol.
	* symtab.c (symbol_matches_domain): New function, takes care
	of dual meaning of STRUCT_DOMAIN symbol for c++, ada and java.
	(lookup_partial_symbol): Use symbol_matches_domain to see if the
	found psym domain matches the given domain.
	(lookup_block_symbol): Likewise.


[-- Attachment #2: duplicatesymbolfix20080502.diff --]
[-- Type: text/plain, Size: 4692 bytes --]

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.258
diff -u -p -r1.258 dwarf2read.c
--- gdb/dwarf2read.c	25 Apr 2008 18:45:23 -0000	1.258
+++ gdb/dwarf2read.c	2 May 2008 13:36:52 -0000
@@ -2060,16 +2060,6 @@ add_partial_symbol (struct partial_die_i
 			   : &objfile->static_psymbols,
 			   0, (CORE_ADDR) 0, cu->language, objfile);
 
-      if (cu->language == language_cplus
-          || cu->language == language_java
-          || cu->language == language_ada)
-	{
-	  /* For C++ and Java, these implicitly act as typedefs as well. */
-	  add_psymbol_to_list (actual_name, strlen (actual_name),
-			       VAR_DOMAIN, LOC_TYPEDEF,
-			       &objfile->global_psymbols,
-			       0, (CORE_ADDR) 0, cu->language, objfile);
-	}
       break;
     case DW_TAG_enumerator:
       add_psymbol_to_list (actual_name, strlen (actual_name),
@@ -7598,23 +7588,16 @@ new_symbol (struct die_info *die, struct
 
 	    /* The semantics of C++ state that "struct foo { ... }" also
 	       defines a typedef for "foo".  A Java class declaration also
-	       defines a typedef for the class.  Synthesize a typedef symbol
-	       so that "ptype foo" works as expected.  */
+	       defines a typedef for the class.  */
 	    if (cu->language == language_cplus
 		|| cu->language == language_java
 		|| cu->language == language_ada)
 	      {
-		struct symbol *typedef_sym = (struct symbol *)
-		  obstack_alloc (&objfile->objfile_obstack,
-				 sizeof (struct symbol));
-		*typedef_sym = *sym;
-		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		/* The symbol's name is already allocated along with
 		   this objfile, so we don't need to duplicate it for
 		   the type.  */
 		if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
 		  TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_SEARCH_NAME (sym);
-		add_symbol_to_list (typedef_sym, list_to_add);
 	      }
 	  }
 	  break;
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.177
diff -u -p -r1.177 symtab.c
--- gdb/symtab.c	19 Apr 2008 11:39:50 -0000	1.177
+++ gdb/symtab.c	2 May 2008 13:36:52 -0000
@@ -1627,6 +1627,26 @@ lookup_symbol_global (const char *name,
 				     domain, symtab);
 }
 
+static int
+symbol_matches_domain (enum language symbol_language, 
+		      domain_enum symbol_domain,
+		      domain_enum domain)
+{
+  /* For c++ "struct foo { ... }" also defines a typedef for "foo".  
+     A Java class declaration also defines a typedef for the class.
+     */
+  if (symbol_language == language_cplus
+      || symbol_language == language_java
+      || symbol_language == language_ada)
+    {
+      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN)
+	return 1;
+    }
+  /* For all other languages, strict match is required.  */
+  return (symbol_domain == domain);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    If LINKAGE_NAME is non-NULL, check in addition that the symbol's
    linkage name matches it.  Check the global symbols if GLOBAL, the
@@ -1691,10 +1711,9 @@ lookup_partial_symbol (struct partial_sy
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
 		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
-	  if (SYMBOL_DOMAIN (*top) == domain)
-	    {
-		  return (*top);
-	    }
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
+				     SYMBOL_DOMAIN (*top), domain))
+	    return (*top);
 	  top++;
 	}
     }
@@ -1706,7 +1725,8 @@ lookup_partial_symbol (struct partial_sy
     {			
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (domain == SYMBOL_DOMAIN (*psym))
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym), 
+				     SYMBOL_DOMAIN (*psym), domain))
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
@@ -1891,7 +1911,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    return sym;
@@ -1912,7 +1933,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    {

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

Daniel Jacobowitz wrote:
> On Sat, Feb 09, 2008 at 12:21:02AM -0500, Aleksandar Ristovski wrote:
>> Hello,
>>
>> Currently we synthesize typedef info for java and c++ (and ada, but I am 
>> not familiar with that language) for any symbol with STRUCT_DOMAIN domain. 
>> This can add up to a lot of redundant data. For partial symbols, we 
>> generate two identical names for the same symbol, once as struct second as 
>> typedef. The same happens with symbols, except we do not duplicate symbol 
>> name, but we do allocate another symbol struture.
>>
>> Instead of generating redundant data for symbols with STRUCT_DOMAIN domain 
>> in java and c++ languages, treat STRUCT_DOMAIN as equal to VAR_DOMAIN for 
>> these two languages when looking up symbols.
> 
> Sorry for taking so long to look at this.  It's a great idea and the
> patch seems correct to me.  Just some cosmetic issues, and then it can
> go in.
> 
>> -      if (cu->language == language_cplus
>> -          || cu->language == language_java
>> -          || cu->language == language_ada)
>> +      if (cu->language == language_ada)
>>  	{
>> -	  /* For C++ and Java, these implicitly act as typedefs as well. */
>> +	  /* FIXME: Check if Ada really
>> +	     needs to implicitly set typedef.  */
> 
> If this works for C++ and Java, it will work for Ada too.  If you
> can't test Ada, please post a final version of the patch and ask Joel
> to run tests for you - I'm sure he won't mind getting all that memory
> back for Ada too.

Ok, here is the revised patch. Cc-ing Joel to take a look if it affects ada.

>> -
>> -	    /* The semantics of C++ state that "struct foo { ... }" also
>> -	       defines a typedef for "foo".  A Java class declaration also
>> -	       defines a typedef for the class.  Synthesize a typedef symbol
>> -	       so that "ptype foo" works as expected.  */
>> +	    
> 
> Since we still need to set the typedef name here, leave the first two
> sentences of the comment.

Sentences put back.

> 
>> +  if (symbol_language == language_cplus
>> +      || symbol_language  == language_java)
> 
> Only need one space there.
> 

One space.


Waiting for an OK to commit.


Thanks,

Aleksandar



ChangeLog:
	* dwarf2read.c (add_partial_symbol): Do not add new psym for
	STRUCT_DOMAIN. Make sure you recognize c++ struct and java and ada 
	class as typedefs. See lookup_partial_symbol function.
	(new_symbol): Similar to add_partial_symbol, do not create 
	symbol for the typedef. See lookup_block_symbol.
	* symtab.c (symbol_matches_domain): New function, takes care
	of dual meaning of STRUCT_DOMAIN symbol for c++, ada and java.
	(lookup_partial_symbol): Use symbol_matches_domain to see if the
	found psym domain matches the given domain.
	(lookup_block_symbol): Likewise.


[-- Attachment #2: duplicatesymbolfix20080502.diff --]
[-- Type: text/plain, Size: 4692 bytes --]

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.258
diff -u -p -r1.258 dwarf2read.c
--- gdb/dwarf2read.c	25 Apr 2008 18:45:23 -0000	1.258
+++ gdb/dwarf2read.c	2 May 2008 13:36:52 -0000
@@ -2060,16 +2060,6 @@ add_partial_symbol (struct partial_die_i
 			   : &objfile->static_psymbols,
 			   0, (CORE_ADDR) 0, cu->language, objfile);
 
-      if (cu->language == language_cplus
-          || cu->language == language_java
-          || cu->language == language_ada)
-	{
-	  /* For C++ and Java, these implicitly act as typedefs as well. */
-	  add_psymbol_to_list (actual_name, strlen (actual_name),
-			       VAR_DOMAIN, LOC_TYPEDEF,
-			       &objfile->global_psymbols,
-			       0, (CORE_ADDR) 0, cu->language, objfile);
-	}
       break;
     case DW_TAG_enumerator:
       add_psymbol_to_list (actual_name, strlen (actual_name),
@@ -7598,23 +7588,16 @@ new_symbol (struct die_info *die, struct
 
 	    /* The semantics of C++ state that "struct foo { ... }" also
 	       defines a typedef for "foo".  A Java class declaration also
-	       defines a typedef for the class.  Synthesize a typedef symbol
-	       so that "ptype foo" works as expected.  */
+	       defines a typedef for the class.  */
 	    if (cu->language == language_cplus
 		|| cu->language == language_java
 		|| cu->language == language_ada)
 	      {
-		struct symbol *typedef_sym = (struct symbol *)
-		  obstack_alloc (&objfile->objfile_obstack,
-				 sizeof (struct symbol));
-		*typedef_sym = *sym;
-		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		/* The symbol's name is already allocated along with
 		   this objfile, so we don't need to duplicate it for
 		   the type.  */
 		if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
 		  TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_SEARCH_NAME (sym);
-		add_symbol_to_list (typedef_sym, list_to_add);
 	      }
 	  }
 	  break;
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.177
diff -u -p -r1.177 symtab.c
--- gdb/symtab.c	19 Apr 2008 11:39:50 -0000	1.177
+++ gdb/symtab.c	2 May 2008 13:36:52 -0000
@@ -1627,6 +1627,26 @@ lookup_symbol_global (const char *name,
 				     domain, symtab);
 }
 
+static int
+symbol_matches_domain (enum language symbol_language, 
+		      domain_enum symbol_domain,
+		      domain_enum domain)
+{
+  /* For c++ "struct foo { ... }" also defines a typedef for "foo".  
+     A Java class declaration also defines a typedef for the class.
+     */
+  if (symbol_language == language_cplus
+      || symbol_language == language_java
+      || symbol_language == language_ada)
+    {
+      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN)
+	return 1;
+    }
+  /* For all other languages, strict match is required.  */
+  return (symbol_domain == domain);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    If LINKAGE_NAME is non-NULL, check in addition that the symbol's
    linkage name matches it.  Check the global symbols if GLOBAL, the
@@ -1691,10 +1711,9 @@ lookup_partial_symbol (struct partial_sy
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
 		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
-	  if (SYMBOL_DOMAIN (*top) == domain)
-	    {
-		  return (*top);
-	    }
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
+				     SYMBOL_DOMAIN (*top), domain))
+	    return (*top);
 	  top++;
 	}
     }
@@ -1706,7 +1725,8 @@ lookup_partial_symbol (struct partial_sy
     {			
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (domain == SYMBOL_DOMAIN (*psym))
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym), 
+				     SYMBOL_DOMAIN (*psym), domain))
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
@@ -1891,7 +1911,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    return sym;
@@ -1912,7 +1933,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    {

Re: [rfc] get rid of redundant data in c++ and java

[Daniel Jacobowitz]

On Fri, May 02, 2008 at 09:39:27AM -0400, Aleksandar Ristovski wrote:
> ChangeLog:
> 	* dwarf2read.c (add_partial_symbol): Do not add new psym for
> 	STRUCT_DOMAIN. Make sure you recognize c++ struct and java and ada  
> 	class as typedefs. See lookup_partial_symbol function.
> 	(new_symbol): Similar to add_partial_symbol, do not create 	symbol for 
> the typedef. See lookup_block_symbol.
> 	* symtab.c (symbol_matches_domain): New function, takes care
> 	of dual meaning of STRUCT_DOMAIN symbol for c++, ada and java.
> 	(lookup_partial_symbol): Use symbol_matches_domain to see if the
> 	found psym domain matches the given domain.
> 	(lookup_block_symbol): Likewise.

Thanks.  Joel, could you give this a try with GNAT?  If it works for
Joel, it's OK to commit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] get rid of redundant data in c++ and java

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

> > 	* dwarf2read.c (add_partial_symbol): Do not add new psym for
> > 	STRUCT_DOMAIN. Make sure you recognize c++ struct and java and ada  
> > 	class as typedefs. See lookup_partial_symbol function.
> > 	(new_symbol): Similar to add_partial_symbol, do not create 	symbol for 
> > the typedef. See lookup_block_symbol.
> > 	* symtab.c (symbol_matches_domain): New function, takes care
> > 	of dual meaning of STRUCT_DOMAIN symbol for c++, ada and java.
> > 	(lookup_partial_symbol): Use symbol_matches_domain to see if the
> > 	found psym domain matches the given domain.
> > 	(lookup_block_symbol): Likewise.
> 
> Thanks.  Joel, could you give this a try with GNAT?  If it works for
> Joel, it's OK to commit.

This is a nice welcome space saver! I missed the patch when it was sent.

There was a piece missing, however. The Ada module has its own bunch
of symbol lookup routines, and some of them were checking the symbol
domain.  So I had to make the symbol_matches_domain routine non-static,
and add a declaration in symtab.h.  I also took the opportunity to fix
an indentation issue, spell "C++" instead of "c++" as well as extend
a comment to explain the Ada situation.

        * ada-lang.c: Update throughout to use symbol_matches_domain
        instead of matching the symbol domain explictly.

I tested the above on x86-linux against the AdaCore testsuite and
the gdb.ada section of our testsuite.

-- 
Joel

[-- Attachment #2: sym-dups.diff --]
[-- Type: text/plain, Size: 7471 bytes --]

Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 130550)
+++ ada-lang.c	(working copy)
@@ -4398,7 +4398,8 @@ ada_lookup_partial_symbol (struct partia
         {
           struct partial_symbol *psym = start[i];
 
-          if (SYMBOL_DOMAIN (psym) == namespace
+          if (symbol_matches_domain (SYMBOL_LANGUAGE (psym),
+                                     SYMBOL_DOMAIN (psym), namespace)
               && wild_match (name, name_len, SYMBOL_LINKAGE_NAME (psym)))
             return psym;
         }
@@ -4432,7 +4433,8 @@ ada_lookup_partial_symbol (struct partia
         {
           struct partial_symbol *psym = start[i];
 
-          if (SYMBOL_DOMAIN (psym) == namespace)
+          if (symbol_matches_domain (SYMBOL_LANGUAGE (psym),
+                                     SYMBOL_DOMAIN (psym), namespace))
             {
               int cmp = strncmp (name, SYMBOL_LINKAGE_NAME (psym), name_len);
 
@@ -4475,7 +4477,8 @@ ada_lookup_partial_symbol (struct partia
         {
           struct partial_symbol *psym = start[i];
 
-          if (SYMBOL_DOMAIN (psym) == namespace)
+          if (symbol_matches_domain (SYMBOL_LANGUAGE (psym),
+                                     SYMBOL_DOMAIN (psym), namespace))
             {
               int cmp;
 
@@ -5593,7 +5596,8 @@ ada_add_block_symbols (struct obstack *o
       struct symbol *sym;
       ALL_BLOCK_SYMBOLS (block, iter, sym)
       {
-        if (SYMBOL_DOMAIN (sym) == domain
+        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+                                   SYMBOL_DOMAIN (sym), domain)
             && wild_match (name, name_len, SYMBOL_LINKAGE_NAME (sym)))
           {
             switch (SYMBOL_CLASS (sym))
@@ -5623,7 +5627,8 @@ ada_add_block_symbols (struct obstack *o
     {
       ALL_BLOCK_SYMBOLS (block, iter, sym)
       {
-        if (SYMBOL_DOMAIN (sym) == domain)
+        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+                                   SYMBOL_DOMAIN (sym), domain))
           {
             int cmp = strncmp (name, SYMBOL_LINKAGE_NAME (sym), name_len);
             if (cmp == 0
@@ -5668,7 +5673,8 @@ ada_add_block_symbols (struct obstack *o
 
       ALL_BLOCK_SYMBOLS (block, iter, sym)
       {
-        if (SYMBOL_DOMAIN (sym) == domain)
+        if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+                                   SYMBOL_DOMAIN (sym), domain))
           {
             int cmp;
 
Index: dwarf2read.c
===================================================================
--- dwarf2read.c	(revision 130550)
+++ dwarf2read.c	(working copy)
@@ -2114,16 +2114,6 @@ add_partial_symbol (struct partial_die_i
 			   : &objfile->static_psymbols,
 			   0, (CORE_ADDR) 0, cu->language, objfile);
 
-      if (cu->language == language_cplus
-          || cu->language == language_java
-          || cu->language == language_ada)
-	{
-	  /* For C++ and Java, these implicitly act as typedefs as well. */
-	  add_psymbol_to_list (actual_name, strlen (actual_name),
-			       VAR_DOMAIN, LOC_TYPEDEF,
-			       &objfile->global_psymbols,
-			       0, (CORE_ADDR) 0, cu->language, objfile);
-	}
       break;
     case DW_TAG_enumerator:
       add_psymbol_to_list (actual_name, strlen (actual_name),
@@ -7863,23 +7853,16 @@ new_symbol (struct die_info *die, struct
 
 	    /* The semantics of C++ state that "struct foo { ... }" also
 	       defines a typedef for "foo".  A Java class declaration also
-	       defines a typedef for the class.  Synthesize a typedef symbol
-	       so that "ptype foo" works as expected.  */
+	       defines a typedef for the class.  */
 	    if (cu->language == language_cplus
 		|| cu->language == language_java
 		|| cu->language == language_ada)
 	      {
-		struct symbol *typedef_sym = (struct symbol *)
-		  obstack_alloc (&objfile->objfile_obstack,
-				 sizeof (struct symbol));
-		*typedef_sym = *sym;
-		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		/* The symbol's name is already allocated along with
 		   this objfile, so we don't need to duplicate it for
 		   the type.  */
 		if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
 		  TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_SEARCH_NAME (sym);
-		add_symbol_to_list (typedef_sym, list_to_add);
 	      }
 	  }
 	  break;
Index: symtab.c
===================================================================
--- symtab.c	(revision 130550)
+++ symtab.c	(working copy)
@@ -1675,6 +1675,26 @@ lookup_symbol_global (const char *name,
 				     domain, symtab);
 }
 
+int
+symbol_matches_domain (enum language symbol_language, 
+		       domain_enum symbol_domain,
+		       domain_enum domain)
+{
+  /* For C++ "struct foo { ... }" also defines a typedef for "foo".  
+     A Java class declaration also defines a typedef for the class.
+     Similarly, any Ada type declaration implicitly defines a typedef.  */
+  if (symbol_language == language_cplus
+      || symbol_language == language_java
+      || symbol_language == language_ada)
+    {
+      if ((domain == VAR_DOMAIN || domain == STRUCT_DOMAIN)
+	  && symbol_domain == STRUCT_DOMAIN)
+	return 1;
+    }
+  /* For all other languages, strict match is required.  */
+  return (symbol_domain == domain);
+}
+
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
    If LINKAGE_NAME is non-NULL, check in addition that the symbol's
    linkage name matches it.  Check the global symbols if GLOBAL, the
@@ -1739,10 +1759,9 @@ lookup_partial_symbol (struct partial_sy
 		 ? strcmp (SYMBOL_LINKAGE_NAME (*top), linkage_name) == 0
 		 : SYMBOL_MATCHES_SEARCH_NAME (*top,name)))
 	{
-	  if (SYMBOL_DOMAIN (*top) == domain)
-	    {
-		  return (*top);
-	    }
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
+				     SYMBOL_DOMAIN (*top), domain))
+	    return (*top);
 	  top++;
 	}
     }
@@ -1754,7 +1773,8 @@ lookup_partial_symbol (struct partial_sy
     {			
       for (psym = start; psym < start + length; psym++)
 	{
-	  if (domain == SYMBOL_DOMAIN (*psym))
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*psym), 
+				     SYMBOL_DOMAIN (*psym), domain))
 	    {
 	      if (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (*psym), linkage_name) == 0
@@ -1939,7 +1959,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    return sym;
@@ -1960,7 +1981,8 @@ lookup_block_symbol (const struct block 
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
-	  if (SYMBOL_DOMAIN (sym) == domain
+	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
+				     SYMBOL_DOMAIN (sym), domain)
 	      && (linkage_name != NULL
 		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
 	    {
Index: symtab.h
===================================================================
--- symtab.h	(revision 130550)
+++ symtab.h	(working copy)
@@ -1019,6 +1019,10 @@ extern const char multiple_symbols_cance
 
 const char *multiple_symbols_select_mode (void);
 
+int symbol_matches_domain (enum language symbol_language, 
+			   domain_enum symbol_domain,
+			   domain_enum domain);
+
 /* lookup a symbol table by source file name */
 
 extern struct symtab *lookup_symtab (const char *);

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

Committed.

Thanks,

Aleksandar

Re: [rfc] get rid of redundant data in c++ and java

[Aleksandar Ristovski]

Committed.

Thanks,

Aleksandar