[webkit-reviews] review granted: [Bug 28701] How many copies of the parameters do you need? : [Attachment 38524] PassRefPtr != RefPtr

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 25 15:10:51 PDT 2009


Darin Adler <darin at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 28701: How many copies of the parameters do you need?
https://bugs.webkit.org/show_bug.cgi?id=28701

Attachment 38524: PassRefPtr != RefPtr
https://bugs.webkit.org/attachment.cgi?id=38524&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    finishParsing(adoptRef(new FunctionParameters(firstParameter)), ident);

The usual idiom would be to make a create function instead of doing the
adoptRef here. I'd prefer to see it done that way.

I'd like to see the same thing for FunctionExecutable too; lets keep those
adoptRef inside create functions, and keep the constructors private.

> +	   void finishParsing(PassRefPtr<FunctionParameters> parameters, const
Identifier& ident);

You could omit both of these argument names.

>  FunctionExecutable::~FunctionExecutable()
>  {
> -    for (int i = 0; i < m_parameterCount; ++i)
> -	   m_parameters[i].~Identifier();
> -    fastFree(m_parameters);
>      delete m_codeBlock;
>  }

Seems like we should use OwnPtr for m_codeBlock.

> +    return adoptRef(new FunctionExecutable(functionName, body->source(),
body->usesArguments(), body->parameters(), body->lineNo(), body->lastLine()));

See above comment about create.

r=me


More information about the webkit-reviews mailing list