Greg, re the const stuff: it sounds like you're advocating a certain
coding policy, but not succeeding in convincing a majority of the
other developers that it's a good policy :-).
Since coding policies only exist to help humans (the code can get
along quite fine without them), if the humans don't find a particular
policy helpful, then there isn't much reason for them to adopt it.
JimB is uncomfortable with reading more into a `const' than the
qualifier technically promises. In the sections of code where he
spends most of his time, it doesn't make sense for him be doing things
that make him uncomfortable -- that'll just get in his way.
My feeling is, you use const the way you like in the code you're
writing, JimB will use it how he likes in the code he's writing, etc,
and Subversion will simply not have a project-wide policy on const
usage.
-K
Jim Blandy <jimb@zwingli.cygnus.com> writes:
> There are two questions here:
>
> - Is const an effective safety mechanism?
>
> If you use const in a way that allows the compiler to verify that
> the invariants you want actually hold, and warn you when they don't,
> then, yes. I'm all in favor of using const this way. If I've
> missed such a case in the code, please point it out.
>
> But if you use const to hint at what you mean --- say, by making
> svn_fs__list_length's argument `const skel_t *', while
> skel->children and skel->next are still mutable --- then it is not
> an effective safety mechanism. If someone adds code which whacks a
> skel node that was supposed to be left alone, the compiler probably
> won't warn you. This half-hearted style of const usage doesn't give
> you the "solid measure of safety" you're going for at all. To the
> contrary, it's more like a thin layer of ice --- who knows whether
> it can hold your weight?
>
> - Should skels be read-only data structures?
>
> I hadn't intended them to be. My thought with skel.[ch] was to lay
> out a simple structure, describe a minimal set of invariants,
> provide some utility functions which expect and preserve those
> invariants, and beyond that, leave it up to the caller to use them
> correctly.
>
> I believe the current code does this. Skels don't fly around willy-
> nilly, as you seem to think. The functions which build atoms from
> literal strings are all in a position to ensure that the skel is
> used correctly. The skel is immediately written to a database
> record; it never passes outside the function, except to the skel
> utility functions, whose behavior is well-known.
>
> I think you're arguing that the entire tree of skel nodes should be
> const --- that is, skel->data, skel->children, and skel->next would
> all refer to const objects, and the code should be changed to use
> constructor-style manipulation. I can see some real advantages to
> this. You'd be shifting from the `hinting to the reader' style to
> thea `compiler can check' style, which addresses the shortcomings
> I've complained about before. The only casting necessary would be
> in the functions that actually construct the skels to begin with,
> which is reasonable.
>
> However, it would make read / modify / write code, like that in
> tree.c:txn_body_change_node_prop, impossible; it would become read /
> reconstruct / write code. Perhaps this isn't a big loss.
>
> In any case, this change should wait until after M2.
>
> Setting the question of skel tree structure aside, I may agree with
> you that skel->data should be const. I can't think of any situation
> where it is appropriate to change *skel->data. Since the atom's text
> is usually a substring in a larger object, you'd have to be dealing
> with fixed-width data, which is pretty rare, and usually a bad idea to
> begin with. This would clean up the string literal issue as well.
Received on Sat Oct 21 14:36:17 2006