On 2014/07/25 at 10:09:41, verwaest wrote:
https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/384963002/diff/200001/src/runtime.cc#newcode9140
src/runtime.cc:9140: PropertyAttributes attrs = UnscopableLookup(&it);
An exception could already have been thrown in UnscopableLookup. Check for
has_pending_exception. I suggest returning (at least)
Maybe<PropertyAttributes>
to indicate this possibility.
https://codereview.chromium.org/384963002/diff/200001/src/unscopables.h
File src/unscopables.h (right):
https://codereview.chromium.org/384963002/diff/200001/src/unscopables.h#newcode28
src/unscopables.h:28: if (!maybe_unscopables.ToHandle(&unscopables))
return
false;
Maybe add a comment here saying that you return false to flag a failure?
OTOH,
what about returning a Maybe<bool> and using that to indicate that you may
have
a failure rather than just a result?
https://codereview.chromium.org/384963002/diff/200001/src/unscopables.h#newcode30
src/unscopables.h:30: return
JSReceiver::HasOwnProperty(Handle<JSReceiver>::cast(unscopables),
This HasOwnProperty can cause an exception at least in the case where the
unscopables is a JSProxy. I wouldn't rely on the returned bool to indicate
whether there is an exception. Check has_pending_exception() instead. (See
my
comment below)
https://codereview.chromium.org/384963002/diff/200001/src/unscopables.h#newcode42
src/unscopables.h:42: while ((attrs =
JSReceiver::GetPropertyAttributes(it))
!= ABSENT) {
At least GetPropertyAttributesWithHandler will return NONE (in at least
some
cases) if GetPropertyAttributes causes an exception to be thrown while
getting
the attributes. GetPropertyAttributes currently has no other way of flagging
that it may fail. The client has to check for has_pending_exception(), and
manually propagate the exception up if so.
(This is obviously very error-prone; and making this explicit is on our
TODO
list).
It seems like exceptions are handled at a higher level already (I'm trying
to
find where). All exceptions I throw at this are being reported as far as I
can
tell.
https://codereview.chromium.org/384963002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
