[webkit-dev] On returning mutable pointers from const methods

[Andreas Kling]

Yo WebKittens!

After some mild morning discussion in #webkit, I'm wondering if we should
amend our style guide to disallow returning mutable pointers (Foo*) from
const methods, like so:

- Foo* foo() const;

While this is useful when you want to be able to take a strong reference to
the returned object, i.e by assigning it to a RefPtr<Foo>, it's
counter-intuitive as it encourages "read-only" methods to hand out
"read/write" references to internal data.

I've been using const-ness to prevent certain classes of bugs at compile
time for the immutable/mutable implementations of WebCore's
ElementAttributeData and StylePropertySet. It's especially important that
the immutable versions of these objects don't have their internal data
modified by unwitting external clients, since they may be shared between
multiple owners. While using assertions helps us catch incorrect use of
these objects, it's obviously optimal to catch the bugs already at compile
time.

I just now discovered that you can grab at a mutable CSSValue* pointer in
an immutable "const StylePropertySet*" by going through
propertyAt(index).value(), since we have "CSSValue* CSSProperty::value()
const". This is bad, because it means that we have a bunch of call sites
operating on mutable pointers into what's supposed to be an immutable
object!

While I haven't identified any real bugs caused by this (yet), it would be
easy to introduce one by mistake.

So, I propose that we allow only these two signature formats for raw
pointers:

- const Foo* foo() const;
- Foo* foo();

Moreover, for methods that return references to objects where call sites
are expected to take a strong reference, we should really be using
PassRefPtr<Foo> as the return type.

Thoughts? Objections? Am I missing something?

-Kling
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121025/1f120d25/attachment.html>