https://codereview.chromium.org/181063008/diff/1/src/mirror-debugger.js
File src/mirror-debugger.js (right):
https://codereview.chromium.org/181063008/diff/1/src/mirror-debugger.js#newcode1941
src/mirror-debugger.js:1941: function ScopeMirror(frame, function,
index, opt_details) {
On 2014/03/04 11:04:19, aandrey wrote:
On 2014/03/04 10:41:59, ulan wrote:
> Maybe just "details" instead of "opt_details". "opt" is confusing,
it probably
> means "optional", but it can also meant "optimized" in v8. :)
Yes, it means "optional", and we do use "opt_" prefix all over the
place (20+
matches in this file).
I think it's fine in this context.
https://codereview.chromium.org/181063008/diff/1/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/181063008/diff/1/src/runtime.cc#newcode12308
src/runtime.cc:12308: // 1: Scope object
Now this seems just a broader case of %GetScopeDetails. Just an idea
that you may dismiss: we could share code and use an unified runtime
function that takes an optional scope index to distinguish between the
two use cases.
https://codereview.chromium.org/181063008/diff/1/src/runtime.cc#newcode12328
src/runtime.cc:12328: ScopeIterator it(isolate, frame,
inlined_jsframe_index);
On 2014/03/04 11:04:19, aandrey wrote:
On 2014/03/04 10:41:59, ulan wrote:
> This code seems to work, but it is fragile because it mixes
handlified code
with
> raw pointers. ScopeIterator can cause GC, so after this point any
raw pointer
to
> v8 heap from above may become invalid.
You mean the JavaScriptFrame* pointer?
I'm not an expert here, but this code, up to the point, is just a
carbon copy of
the above Runtime_GetScopeDetails.
The JavaScriptFrame class is BASE_EMBEDDED. Should it care on GC being
run?
Seems fine to me.
https://codereview.chromium.org/181063008/diff/1/src/runtime.cc#newcode12330
src/runtime.cc:12330: MaybeObject* maybe_object =
MaterializeScopeDetails(isolate, &it);
On 2014/03/04 11:04:19, aandrey wrote:
On 2014/03/04 10:41:59, ulan wrote:
> MaterializeScopeDetails looks already handlified, it could just
return a
handle,
> instead of maybe_objects. The you wouldn't need checks and
conversions below.
As far as I understand, it may return a Failure which inherits
MaybeObject and
not Object, thus the return type is MaybeObject*.
What syntax would you suggest for MaterializeScopeDetails?
I suggest changing MaterializedScopeDetails to return a
Handle<JSObject>. If the scope_object is an empty handle, use
RETURN_IF_EMPTY_HANDLE_VALUE to propagate that empty handle to the
caller. The caller should then check with RETURN_IF_EMPTY_HANDLE.
I know it's not actually caused by this CL, but I since we are moving
towards less code with MaybeObject, this would be a nice small clean up.
https://codereview.chromium.org/181063008/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
