[webkit-dev] Lets use PassRefPtr for arguments less; lets use RefPtr for locals and data members more

David Levin levin at chromium.org
Mon Jun 20 17:43:42 PDT 2011


On Mon, Jun 20, 2011 at 5:25 PM, Maciej Stachowiak <mjs at apple.com> wrote:

>
> On Jun 20, 2011, at 9:19 AM, Alexey Proskuryakov wrote:
>
>
> 20.06.2011, в 03:22, Maciej Stachowiak написал(а):
>
> For a shared ownership model there are multiple possible definitions of
> whether a function takes ownership to an object passed as an argument. Here
> are some of my attempts to describe the bright line:
>
>    a) Hands off ownership to what could possibly be the sole owner in most
> code paths.
>    b) Keeps a reference to the object after the function completes in most
> code paths.
>    c) Takes a reference to the object at least once in most code paths.
>
>    d) Hands off ownership to what could possibly be the sole owner in some
> code paths.
>    e) Keeps a reference to the object after it completes in some code
> paths.
>    f) Takes a reference to the object at least once in some code paths.
>
> Is the bright line rule you have in mind (b) or perhaps (e)? Or something
> not listed here at all?
>
>
> Yes, (b) or (e). I haven't thought about whether "most code paths" or "some
> code paths" makes more sense, but I'm not sure there are a lot of cases in
> our code where it makes a difference.
>
> I don't think it makes sense to talk about a sole owner of a refcounted
> object. If there is a sole owner at any given time, it is at best temporary.
> PassRefPtr is about clearly and efficiently transferring a reference, it
> doesn't need it to necessarily be the sole then-existing reference.
>
>
> I think that to make this complete, the rules need to be transitive. A
> function that passes its argument to another function taking a PassRefPtr
> should itself take a PassRefPtr. That's the case in <
> https://bugs.webkit.org/show_bug.cgi?id=52981>, for instance.
>
> It doesn't seem that analyzing code for "most/some code paths takes
> ownership" would be any easier that analyzing it for "any caller gives away
> ownership".
>
>
> In general it only requires inspecting the source of the function, and
> possibly the signatures of function it calls. The analysis for "any caller
> gives away ownership" requires searching over all of WebCore to find
> possible call sites. And it can easily change
>
> If we think the transitivity problem is a hazard, we could make PassRefPtr
> refuse to implicitly convert from raw pointers. Then to pass a raw pointer
> to a function that uses PassRefPtr you'd have to make a RefPtr first (or
> adopt, if the ref is unowned). That would make "takes ownership" analysis
> purely local to the function source.
>

I thought the problem was that a Pass*Ptr could silently 0 itself out and
people use it directly.

So the real problem functions are anything the fact that a Pass*Ptr can be
used like a pointer and that another Pass*Ptr can slurp out its contents so
easily (and you can't spot this in the code well).

Maybe each of those operations should be more explicit? (Then we could
probably even catch these issues automatically. If *.passPtr() is called and
then *.get() is called later in the same function, mark it as an error. Not
perfect but good enough.)



> Yet it's the latter where PassRefPtr is beneficial. Why base the rule on
> something that's disconnected from actual benefit?
>
>
> Because it's simpler to read the source of your own function than to visit
> all call sites, and it's more obvious that when you change what the function
> does you may need to change the signature.
>

This seems much easier, less error prone, and more stable.

dave


> Regards,
> Macij
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110620/d865d7ba/attachment.html>


More information about the webkit-dev mailing list