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

[Geoffrey Garen]

Responding to a few comments at once:

> I'm curious if there was a specific patch or piece of code that lead you to send this email out - perhaps we make a better decision about whether to change our approach with more concrete examples of problems the current situation has caused or is causing.

In https://bugs.webkit.org/show_bug.cgi?id=59604, Simon made a general comment that new methods should use const, but Alexey disagreed. Then I began this discussion, in the hopes of resolving the issue and writing a clear style guideline.

I agree that concrete code is easier to discuss than abstract principles. Still, I'd like to write a general style guideline that coders and reviewers can use.

On the specific question of accessors, it sounds like there's some consensus that accessors should be const, unless they return pointers or references, in which case accessors should be non-const, or should return const pointers or const references. In some cases, this may require two copies of the same accessor, but we'd rather not provide two copies of every accessor. 

Is that a fair summary?

>> What are some good examples of existing code that meaningfully uses a const pointer or reference? (Something other than, say, the obligatory const& in a copy constructor.)

> 
> I personally find that in a number of render tree functions declared to take a const RenderObject* that the const-ness is a useful hint that the function will not be manipulating the object in unexpected ways.  Examples:
> http://codesearch.google.com/codesearch?hl=en&vert=chromium&lr=&q=%22const+RenderObject%22+file%3AWebCore&sbtn=Search (click "Next" at the bottom to see more results).
> 
> I think const reference to String and const pointers to CSSStyleDeclaration/CSSMutableStyleDeclaration are good examples.
> 
> All functions passed to enclosingNodeOfType in htmlediting.cpp are such clients:

Thanks, this is a good set of examples.

> Perhaps we could at least encourage const-correctness for newly-written classes? By this I mean both adherence to the logical-constness rules you stated earlier as well as not adding non-const accessors and members unless needed -- otherwise it's easy to just err on the side of never using const anywhere.

I'm still not convinced that it's meaningful to talk about a "const-correct class". The const that you put on a member function only has meaning when somebody uses a const pointer or reference to your class. More importantly, any design issues raised or bugs caught by const only get tested when somebody uses a const pointer or reference to your class.

So, the key to const-correctness is deciding when to use const pointers and const references -- once you're using a const pointer or reference, the compiler will tell you which member functions need to be const.

> I also think we should not discourage people from using "const" on variables.  I've gotten review comments in the past that have asked me to remove const qualifiers on locals, as well as sometimes on arguments and members, and I think this is a mistake.

I tend to agree. On a project-wide scale, it's an obvious contradiction, and a bit three-stooges-esque, for reviewers to ask for more const member function declarations and fewer const variable declarations. There is no const-correctness without const variable declarations.

That said, I think const is most useful when used with a pointer or a reference, because it provides for a separation of concerns, and acts as a barrier between logically independent pieces of code that live in different source files but share data. Perhaps a data member also falls into this category, since it can be modified by any member function. In contrast, declaring a local int const will only protect you from contradicting your own intentions inside one block scope -- to me, that's not really worth the extra verbosity.

> However, in the interests of pragmatism I think that it would be reasonable to at least remove the improper uses of const (b).

OK, it seems that there is also some consensus toward removing the const qualifier from member functions that return non-const pointers or references, or otherwise fail to meet the logical constness test.

> From the tone of the initial e-mail it sounded like there was some desire to get rid of const declarations across the board.  I would be opposed to this change.


I think I may have gone a little too far in venting my own confusion about the role of const member functions in C++ -- to stimulate discussion, I made a strawperson proposal not to use const member functions at all, but I don't think anyone actually supports that proposal, even though there is some support for removing incorrect or unhelpful uses of const member functions.

Geoff
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110602/d88b224f/attachment.html>