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

[Stephen Chenney]

I second the principle. I ran into this kind of code pattern recently in
fonts and was unnerved by the thought of a mutable pointer emerging from a
const method.

I am not at all certain that we can simply make the change without some
heavy refactoring. On the one hand, there are some long const-ness chains
in some places that removing const from a method will break. On the other
hand, making the returned pointer const in some places will force
additional const-ness or require changes to code that uses the pointer.

Do we try to apply the new style to new code only?

Stephen.

On Thu, Oct 25, 2012 at 6:48 AM, Andreas Kling <kling at webkit.org> wrote:

> 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
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121025/5a6ad447/attachment.html>