On Mon, Jul 11, 2011 at 07:57:09PM +0200, Clemens Buchacher wrote:
> On Mon, Jul 11, 2011 at 12:18:14PM -0400, Jeff King wrote:
> >
> > +unsigned long commit_generation(const struct commit *commit)
> > +{
> [...]
> > + /* drop const because we may call parse_commit */
> > + return commit_generation_recurse((struct commit *)commit);
> > +}
>
> Out of curiosity, why make it const in the first place?
Two reasons:
1. At the API layer, it's conceptually const. We're not changing the
commit, but the lazy load of parse_commit is an implementation
detail.
In C++, you would stick a "mutable" tag on the parts we lazily load
via parse_object. Here, we have to cast.
This isn't C++, of course, and while we do follow some
object-oriented principles, it's not necessarily worth fighting the
language like this for the sake of a const. And I would be fine
with saying "all commit objects should not be marked const, because
we may lazily parse them, and it's well known that they are not to
be freed anyway".
But...
2. The callsite in pretty.c has a const commit, so we have to cast
somewhere, and this spot seemed the most appropriate to me (or we
could drop the consts there, which I would be OK with).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html