Remove calls to SetLocalPropertyIgnoreAttributesTrampoline from accessors.cc

Remove calls to SetLocalPropertyIgnoreAttributesTrampoline from accessors.cc

BUG=v8:2877
R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=17489

[Michael Starzinger, 2013-11-05 12:48:17]

Looking good, just one real comment.

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode178
src/accessors.cc:178: return *JSObject::SetLocalPropertyIgnoreAttributes(object,
I think it's possible that setting the property fails due to an exception (e.g.
extension of "object" has been prevented). In that case the returned handle
might be empty and the dereference will fail. We should use the following
pattern ...

Handle<Object> result = JSObject::SetLocalPropertyIgnoreAttributes();
RETURN_IF_EMPTY_HANDLE(isolate, result);
return *result;

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode184
src/accessors.cc:184: Handle<JSArray>
array_handle(Handle<JSArray>::cast(object));
nit: Better use assignment instead of constructor syntax here for consistency
with the rest of the code ...

Handle<JSArray> array_handle = Handle<JSArray>::cast(object);

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode595
src/accessors.cc:595: return *JSObject::SetLocalPropertyIgnoreAttributes(object,
Likewise. See the comment above about the possibility of an exception.

[rafaelw, 2013-11-05 13:35:15]

PTAL

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode178
src/accessors.cc:178: return *JSObject::SetLocalPropertyIgnoreAttributes(object,
On 2013/11/05 12:48:17, Michael Starzinger wrote:
> I think it's possible that setting the property fails due to an exception
(e.g.
> extension of "object" has been prevented). In that case the returned handle
> might be empty and the dereference will fail. We should use the following
> pattern ...
> 
> Handle<Object> result = JSObject::SetLocalPropertyIgnoreAttributes();
> RETURN_IF_EMPTY_HANDLE(isolate, result);
> return *result;

Done.

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode184
src/accessors.cc:184: Handle<JSArray>
array_handle(Handle<JSArray>::cast(object));
On 2013/11/05 12:48:17, Michael Starzinger wrote:
> nit: Better use assignment instead of constructor syntax here for consistency
> with the rest of the code ...
> 
> Handle<JSArray> array_handle = Handle<JSArray>::cast(object);

Done.

https://codereview.chromium.org/59773002/diff/30001/src/accessors.cc#newcode595
src/accessors.cc:595: return *JSObject::SetLocalPropertyIgnoreAttributes(object,
On 2013/11/05 12:48:17, Michael Starzinger wrote:
> Likewise. See the comment above about the possibility of an exception.

Done.

[Michael Starzinger, 2013-11-05 13:37:35]

LGTM.

[rafaelw, 2013-11-05 13:47:59]

Committed patchset #4 manually as r17489 (presubmit successful).