Re: [PATCH v2 3/5] Change meaning of VALUE_FRAME_ID; rename to VALUE_NEXT_FRAME_ID

[Kevin Buettner <kevin@buettner.to>]

On Wed, 9 Nov 2016 14:48:35 +0000
Pedro Alves <palves@redhat.com> wrote:

> On 11/02/2016 10:19 PM, Kevin Buettner wrote:
> 
> > @@ -2295,7 +2305,10 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
> >    if (ctx.num_pieces > 0)
> >      {
> >        struct piece_closure *c;
> > -      struct frame_id frame_id = get_frame_id (frame);
> > +      struct frame_id frame_id
> > +        = ((frame == NULL)   
> 
> Redundant parens.

Fixed.

> > +	   We use VALUE_FRAME_ID for obtaining the value's frame id instead of
> > +	   VALUE_NEXT_FRAME_ID due to requiring a frame which may be passed  to  
> 
> Spurious space in "passed  to".

Fixed.

> > +	   put_frame_register_bytes() below.  That function will (eventually)
> > +	   perform the any necessary unwind operation by first obtaining the next
> > +	   frame.  */  
> 
> "the any necessary" looks like a typo?

Agreed.  I removed "any " from the comment.

> The comment just below needs updating: it's still phrased in terms
> of get_frame_register_value.  Also, I suspect that renaming the "frame" and
> "frame_id" locals to "next_frame" and "next_frame_id" would allow simplifying
> the new comment.
> 
> >  
> >  	  /* If we get another lazy lval_register value, it means the
> >  	     register is found by reading it from the next frame.

I made the next_frame and next_frame_id changes that you suggested.
Here's the change for the comment:

 	  /* If we get another lazy lval_register value, it means the
-	     register is found by reading it from the next frame.
-	     get_frame_register_value should never return a value with
-	     the frame id pointing to FRAME.  If it does, it means we
+	     register is found by reading it from NEXT_FRAME's next frame.
+	     frame_unwind_register_value should never return a value with
+	     the frame id pointing to NEXT_FRAME.  If it does, it means we
 	     either have two consecutive frames with the same frame id
 	     in the frame chain, or some code is trying to unwind
 	     behind get_prev_frame's back (e.g., a frame unwind

> Otherwise LGTM.

Thanks again for the review.

Pushed.

Kevin