On Tue, 2014-04-15 at 14:24 -0700, Cary Coutant wrote:
> > +       /* If HOST_WIDE_INT is big enough then represent the bound as
> > +          a constant value.  Note that we need to make sure the type
> > +          is signed or unsigned.  We cannot just add an unsigned
> > +          constant if the value itself is positive.  Some DWARF
> > +          consumers will lookup the bounds type and then sign extend
> > +          any unsigned values found for signed types.  This is only
> > +          for DW_AT_lower_bound, normally unsigned values
> > +          (DW_FORM_data[1248]) are assumed to not need
> > +          sign-extension.  */
> 
> This comment confuses me.

Sorry, obviously not my intention. But I see what I was trying to say
and how I said it didn't make things very clear. Apologies.

>  By "we need to make sure the type is signed
> or unsigned" (what else can it be?), I think you mean "we need to
> choose a form based on whether the type is signed or unsigned."

Yes, right. I was confusing matters in my comment because I was thinking
of non-constants (reference or exprlocs) that are handled elsewhere
later on in the code.

>  And by "This is only for DW_AT_lower_bound, ...", I think you mean "This is
> needed only for DW_AT_{lower,upper}_bound, since for most other
> attributes, consumers will treat DW_FORM_data[1248] as unsigned
> values, regardless of the underlying type."

Yes, right again.

> Otherwise, the patch looks OK to me.

Thanks I pushed it with the comment changed to how you expressed things.
It now reads:

/* If HOST_WIDE_INT is big enough then represent the bound as           
   a constant value.  We need to choose a form based on                 
   whether the type is signed or unsigned.  We cannot just              
   call add_AT_unsigned if the value itself is positive                 
   (add_AT_unsigned might add the unsigned value encoded as             
   DW_FORM_data[1248]).  Some DWARF consumers will lookup the           
   bounds type and then sign extend any unsigned values found           
   for signed types.  This is needed only for                           
   DW_AT_{lower,upper}_bound, since for most other attributes,          
   consumers will treat DW_FORM_data[1248] as unsigned values,          
   regardless of the underlying type.  */

Thanks,

Mark

Reply via email to