Re: Do we have a style preference about const member functions?
Maciej Stachowiak <mjs <at> apple.com>
2011-06-01 06:31:49 GMT
On May 31, 2011, at 10:00 PM, Brent Fulgham wrote:
>
> On May 31, 2011, at 8:44 PM, Maciej Stachowiak wrote:
>
>> For example, the compiler does not tell you that the following implementation of
Node::previousSibling() (currently in our code!) is totally wrong from the "logical const" perspective:
>>
>> Node* previousSibling() const { return m_previous; }
>>
>> The correct way to do const-correctness here would require more verbosity:
>>
>> const Node* previousSibling() const { return m_previous; }
>> Node* previousSibling() { return m_previous; }
>>
>> And the first variant would be dead code if we don't ever actually use const node pointers.
>>
>> Given your views on const-correctness discipline, what do you think is the right way to handle this
situation? Note that this pattern comes up for many core DOM methods and many render tree methods.
>
>
> One possible (though ugly) way of allowing the compiler to do some of this work for you is to declare the
m_previous member as a const pointer, and then manipulate it only in specific routines using the
"const_cast" operator to allow it to mutate. But this is probably a case of the cure being worse than the disease.
This cure would most definitely be worse than the disease. All JavaScript access to the DOM gets
(effectively) non-const node references, so we'd need to cast away const everywhere.
>
> If we had logic that iterated over the node siblings in read-only fashion, we would ideally do so through a
const iterator. In addition to documenting that we don't intend to mutate the object, it would provide
potentially useful information that compilers could use to make aliasing decisions and so forth.
Perhaps we never iterate over DOM nodes without intending to mutate them; if so, I would agree that there is
no benefit to maintaining the const variation.
It's my understanding that compilers make essentially no use of a method being labeled "const" to enable
optimizations. Given const_cast and mutable, just knowing the signature is const is not a strong enough
guarantee to decide anything at the call site. And if the compilercan see the body of the function, and see
that it actually doesn't mutate the object, it know all it needs to know and are not helped by the const
declaration. Marking methods as const to help the compiler is, as far as I know, simply cargo cult engineering.
>
> However I do not think (as the Node example might imply) that the fact that the compiler cannot identify ALL
categories of const error means that we are better off washing our hands of const correctness.
Let's set aside the compiler's error checking for the moment. What do you believe is the proper
const-correct way to write previousSibling() and related methods? A priori, I think the correct way is this:
Node* previousSibling() { return m_previous; }
I could also be convinced that the following is technically more correct, though in a way that is completely
useless for our code base at present:
const Node* previousSibling() const { return m_previous; }
Node* previousSibling() { return m_previous; }
The following would be valid but would require us to cast away const all over the place and would therefore in
my opinion be a net negative:
const Node* previousSibling() const { return m_previous; }
And what's in our code now violates the intent of const, and so to me at least is clearly a bug:
Node* previousSibling() const { return m_previous; }
What do you think is the right way to do it? One of these? Something else?
> In fact, due to the viral nature of const-ness changes, leaving them in (and continuing to maintain them)
is a good long term approach since the first time you want to work with a const Node object you will have to
resurrect const variations of methods across the object hierarchy.
Well one big problem right now (just from scanning the core DOM classes) is that we have a lot of clearly
broken use of const. We could (a) leave it as-is, (b) remove the incorrect use of const, or (c) deploy proper
const-correctness. it seems like you are against (b), but I cannot tell if you advocate (a) or (c).
Regards,
Maciej
RSS Feed