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

[Aleksandar Ristovski <aristovski@qnx.com>]

[-- 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))
 	    {