[webkit-dev] Do we have a style preference about const member functions?

[Maciej Stachowiak]

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