[llvm-commits] [PATCH] MCFragments Clean Up

[Rafael Espíndola]

2010/11/2 David Meyer <pdox at google.com>:
> Rafael,
> If I didn't do this, ComputeSize() would need two more arguments:
> SectionAddress and FragmentOffset. MCAlignFragment uses SectionAddress +
> FragmentOffset, while MCOrgFragment uses FragmentOffset. I was trying to
> avoid cluttering up the function arguments with specific values needed for
> one or two fragment types. Since we have to pass the MCAsmLayout anyway, it
> seems reasonable that it should be capable of answering those values for the
> current fragment (since they are guaranteed to be known).
> In order to avoid passing FragmentOffset, your patch uses private member
> "Offset" directly, which is fragile (what if it is uninitialized?) and
> against the convention of using the accessor. Also, this won't work when you
> move the code away from MCFragment and into the subclasses. (Offset is a
> private member of MCFragment).

The information is an "argument". It can be made explicit or implicit
via some value in this->. Note also that you only need SectionAddress
(see patch attached two emails ago). As for being private, I think it
is reasonable to make it protected as you are moving responsibility
from MCFragment itself to it its subclasses.

> - David M
>

Cheers,
Rafael