[David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[David Carlton]

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

Here's a ping for a patch that I'd like to get in before the 5.3
branch is cut; I'm sending the ping now even though I originally sent
the patch out just under a week ago because I'll be out of the office
through next Monday.  (I'm not this pushy normally, honest.)

The reason why I want to get this into the 5.3 branch is because it
fixes a seg fault and seg faults make me unduly nervous. :-)

David Carlton
carlton@math.stanford.edu


[-- Attachment #2: Type: message/rfc822, Size: 5170 bytes --]

From: David Carlton <carlton@math.Stanford.EDU>
To: Andrew Cagney <ac131313@ges.redhat.com>
Cc: Daniel Jacobowitz <drow@mvista.com>, gdb-patches@sources.redhat.com
Cc: carlton@math.Stanford.EDU
Subject: Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods
Date: 22 Aug 2002 14:24:16 -0700
Message-ID: <ro1elcqpoxr.fsf@jackfruit.Stanford.EDU>

In article <3D655311.6030603@ges.redhat.com>, Andrew Cagney <ac131313@ges.redhat.com> writes:

> The place to put comments explaining changes is in the source
> code.
...
> The ChangeLog need only state what changed, not why.

Whoops, sorry about that.  How about this patch?

By the way, am I correct in thinking that the ChangeLog is supposed
to mention the PR number?  I noticed that GNATS picked up some of my
earlier changes, and I'm assuming it noticed the PR number from the
log message.

David Carlton
carlton@math.stanford.edu

2002-08-22  David Carlton  <carlton@math.stanford.edu>

	* dwarf2read.c (dwarf2_add_member_fn): Add the 'type'
	argument (PR gdb/653).  Update call to smash_to_method_type.
	(read_structure_scope): Update call to dwarf2_add_member_fn.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.65
diff -u -p -r1.65 dwarf2read.c
--- dwarf2read.c	20 Aug 2002 18:45:30 -0000	1.65
+++ dwarf2read.c	22 Aug 2002 21:22:51 -0000
@@ -803,7 +803,8 @@ static void dwarf2_attach_fields_to_type
 					  struct type *, struct objfile *);
 
 static void dwarf2_add_member_fn (struct field_info *,
-				  struct die_info *, struct objfile *objfile,
+				  struct die_info *, struct type *,
+				  struct objfile *objfile,
 				  const struct comp_unit_head *);
 
 static void dwarf2_attach_fn_fields_to_type (struct field_info *,
@@ -2259,7 +2260,7 @@ dwarf2_attach_fields_to_type (struct fie
 
 static void
 dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
-		      struct objfile *objfile,
+		      struct type *type, struct objfile *objfile,
 		      const struct comp_unit_head *cu_header)
 {
   struct attribute *attr;
@@ -2327,7 +2328,15 @@ dwarf2_add_member_fn (struct field_info 
       struct type *return_type = TYPE_TARGET_TYPE (die->type);
       int nparams = TYPE_NFIELDS (die->type);
 
-      smash_to_method_type (fnp->type, die->type,
+      /* NOTE: carlton/2002-08-22: Previously, the second argument to
+	 smash_to_method_type was die->type rather than type, and the
+	 type argument to dwarf2_add_member_fn didn't exst.  This is
+	 incorrect: the second argument to smash_to_method_type should
+	 be the type of the class that this is a method of, whereas
+	 die->type is the type of the method itself.  So we need to
+	 pass that type in from read_structure_scope explicitly.  See
+	 PR gdb/653.  */
+      smash_to_method_type (fnp->type, type,
 			    TYPE_TARGET_TYPE (die->type),
 			    TYPE_FIELDS (die->type),
 			    TYPE_NFIELDS (die->type),
@@ -2516,7 +2525,7 @@ read_structure_scope (struct die_info *d
 	    {
 	      /* C++ member function. */
 	      process_die (child_die, objfile, cu_header);
-	      dwarf2_add_member_fn (&fi, child_die, objfile, cu_header);
+	      dwarf2_add_member_fn (&fi, child_die, type, objfile, cu_header);
 	    }
 	  else if (child_die->tag == DW_TAG_inheritance)
 	    {

Re: [David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[Daniel Jacobowitz]

On Wed, Aug 28, 2002 at 03:25:50PM -0700, David Carlton wrote:
> Here's a ping for a patch that I'd like to get in before the 5.3
> branch is cut; I'm sending the ping now even though I originally sent
> the patch out just under a week ago because I'll be out of the office
> through next Monday.  (I'm not this pushy normally, honest.)
> 
> The reason why I want to get this into the 5.3 branch is because it
> fixes a seg fault and seg faults make me unduly nervous. :-)

My judgement would be that this fixes an obvious mistake in my previous
patch.


> @@ -2327,7 +2328,15 @@ dwarf2_add_member_fn (struct field_info 
>        struct type *return_type = TYPE_TARGET_TYPE (die->type);
>        int nparams = TYPE_NFIELDS (die->type);
>  
> -      smash_to_method_type (fnp->type, die->type,
> +      /* NOTE: carlton/2002-08-22: Previously, the second argument to
> +	 smash_to_method_type was die->type rather than type, and the
> +	 type argument to dwarf2_add_member_fn didn't exst.  This is
> +	 incorrect: the second argument to smash_to_method_type should
> +	 be the type of the class that this is a method of, whereas
> +	 die->type is the type of the method itself.  So we need to
> +	 pass that type in from read_structure_scope explicitly.  See
> +	 PR gdb/653.  */
> +      smash_to_method_type (fnp->type, type,
>  			    TYPE_TARGET_TYPE (die->type),
>  			    TYPE_FIELDS (die->type),
>  			    TYPE_NFIELDS (die->type),

I'd rather a comment like:
	/* TYPE is the domain of this method, and DIE->TYPE is the type
	   of the method itself (TYPE_CODE_METHOD).  */

There's no point in cluttering up the code with history of this sort
unless you have low confidence in the change's effect on some odd
corner-case.  That's just my personal judgement, though.

If you agree, mind committing it with that or a similar change?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[David Carlton]

On Wed, 28 Aug 2002 18:42:47 -0400, Daniel Jacobowitz <drow@mvista.com> said:

>> +      /* NOTE: carlton/2002-08-22: Previously, the second argument to
>> +	 smash_to_method_type was die->type rather than type, and the
>> +	 type argument to dwarf2_add_member_fn didn't exst.  This is
>> +	 incorrect: the second argument to smash_to_method_type should
>> +	 be the type of the class that this is a method of, whereas
>> +	 die->type is the type of the method itself.  So we need to
>> +	 pass that type in from read_structure_scope explicitly.  See
>> +	 PR gdb/653.  */

> I'd rather a comment like:
> 	/* TYPE is the domain of this method, and DIE->TYPE is the type
> 	   of the method itself (TYPE_CODE_METHOD).  */

> There's no point in cluttering up the code with history of this sort
> unless you have low confidence in the change's effect on some odd
> corner-case.  That's just my personal judgement, though.

I agree.  I'd originally put in the excessively verbose comment
because Andrew complained about my putting too much information in the
ChangeLog instead of a comment, but I think that your version of the
comment is better.

> If you agree, mind committing it with that or a similar change?

Great, will do (unless Andrew complains about the new version of the
comment).

David Carlton
carlton@math.stanford.edu

Re: [David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[Andrew Cagney]

> On Wed, 28 Aug 2002 18:42:47 -0400, Daniel Jacobowitz <drow@mvista.com> said:
> 
> 
>>> +      /* NOTE: carlton/2002-08-22: Previously, the second argument to
>>> +	 smash_to_method_type was die->type rather than type, and the
>>> +	 type argument to dwarf2_add_member_fn didn't exst.  This is
>>> +	 incorrect: the second argument to smash_to_method_type should
>>> +	 be the type of the class that this is a method of, whereas
>>> +	 die->type is the type of the method itself.  So we need to
>>> +	 pass that type in from read_structure_scope explicitly.  See
>>> +	 PR gdb/653.  */
> 
> 
>> I'd rather a comment like:
>> 	/* TYPE is the domain of this method, and DIE->TYPE is the type
>> 	   of the method itself (TYPE_CODE_METHOD).  */
> 
> 
>> There's no point in cluttering up the code with history of this sort
>> unless you have low confidence in the change's effect on some odd
>> corner-case.  That's just my personal judgement, though.
> 
> 
> I agree.  I'd originally put in the excessively verbose comment
> because Andrew complained about my putting too much information in the
> ChangeLog instead of a comment, but I think that your version of the
> comment is better.
> 
> 
>> If you agree, mind committing it with that or a similar change?
> 
> 
> Great, will do (unless Andrew complains about the new version of the
> comment).

Doesn't worry me.

Andrew

Re: [David Carlton <carlton@math.Stanford.EDU>] Re: [RFA] dwarf2read.c: set TYPE_DOMAIN_TYPE correctly for methods

[David Carlton]

On Wed, 28 Aug 2002 18:42:47 -0400, Daniel Jacobowitz <drow@mvista.com> said:

> If you agree, mind committing it with that or a similar change?

Done, with your version of the comment.

David Carlton
carlton@math.stanford.edu