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