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 <[email protected]> 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 > [email protected] > http://lists.webkit.org/mailman/listinfo/webkit-dev > >
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo/webkit-dev
