Yo WebKittens!<div><br></div><div>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:</div><div><br>
</div><div>- Foo* foo() const;</div><div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div>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!</div>
<div><br></div><div>While I haven't identified any real bugs caused by this (yet), it would be easy to introduce one by mistake.</div><div><br></div><div>So, I propose that we allow only these two signature formats for raw pointers:</div>
<div><br></div><div><div>- const Foo* foo() const;</div><div>- Foo* foo();</div><div><br></div><div>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.</div>
<div><br></div></div><div>Thoughts? Objections? Am I missing something?</div><div><br></div><div>-Kling</div>