Looking good, just minor comments.
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode84
src/compiler.cc:84: explicit
CompilationInfoWithZone(Handle<SharedFunctionInfo> shared)
See comment below.
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1001
src/compiler.cc:1001: CompilationInfoWithZone info(shared);
There is a constant back-and-forth refactoring around
CompilationInfoWithZone. Can we make the following lines explicit here
to avoid adding a new constructor:
Zone zone;
ParseInfo parse_info(&zone, shared);
CompilationInfo info(&parse_info);
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1394
src/compiler.cc:1394: allow_lazy &=
!LiveEditFunctionTracker::IsActive(isolate);
nit: Can we combine this into one big conjunction instead?
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1464
src/compiler.cc:1464: DCHECK(info.is_debug() ? !existing->HasDebugCode()
nit: Not sure exactly what this assertion is guarding against, I would
be fine with dropping it entirely.
https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1560
src/compiler.cc:1560: DCHECK(!shared->HasDebugInfo());
What happens of a concurrent compile job for a function has been
enqueued and the debugger sets a breakpoint inside that function while
it's on the queue. Does the debugger flush the queue somehow? Or is
there another mechanism in place to prevent this?
https://codereview.chromium.org/1233073005/diff/1/src/compiler.h
File src/compiler.h (right):
https://codereview.chromium.org/1233073005/diff/1/src/compiler.h#newcode630
src/compiler.h:630: MUST_USE_RESULT static MaybeHandle<Code>
CompileForDebugging(
What do you think about sticking with GetDebugCode as the name for both
of these function, to keep it in sync with the rest of the API methods?
https://codereview.chromium.org/1233073005/diff/1/src/compiler/js-inlining.cc
File src/compiler/js-inlining.cc (right):
https://codereview.chromium.org/1233073005/diff/1/src/compiler/js-inlining.cc#newcode254
src/compiler/js-inlining.cc:254: return NoChange();
Please add a tracing line ...
TRACE("Not inlining %s into %s because callee contains breakpoints\n",
...)
https://codereview.chromium.org/1233073005/
--
--
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/d/optout.
