LGTM with a couple of suggestions.
http://codereview.chromium.org/8050010/diff/2001/src/compiler.cc File src/compiler.cc (right): http://codereview.chromium.org/8050010/diff/2001/src/compiler.cc#newcode314 src/compiler.cc:314: if (info->CompilingForDebugging()) { You could piggyback on the existing test so the control flow through here doesn't get too complicated conditions don't get too complicated: return V8::UseCrankshaft() && !info->CompilingForDebugging() ? MakeCrankshaftCode(info) : FullCodeGenerator::MakeCode(info); or return info->CompilingForDebugging() || !V8::UseCrankshaft() ? FullCodeGenerator::MakeCode(info) : MakeCrankshaftCode(info); http://codereview.chromium.org/8050010/diff/2001/src/compiler.h File src/compiler.h (right): http://codereview.chromium.org/8050010/diff/2001/src/compiler.h#newcode250 src/compiler.h:250: bool compiling_for_debugging_; There's a bit field "flags_" in this class. I think this bit (as well as supports_deoptimization_) should be packed in there. Conveniently, the flags all default to false. http://codereview.chromium.org/8050010/diff/2001/src/debug.cc File src/debug.cc (right): http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1734 src/debug.cc:1734: static int HandleObjectPointerCompare(const Handle<T>* a, const Handle<T>* b) { Hmm. There are a couple of other Compare functions in utils.h. I don't think this can go there, because of include dependencies, but maybe it should go in handles.h with a comment in utils.h that it's in handles.h, and a comment in handles.h that it's like the ones in utils.h but moved because of include dependencies. I'm worried someone else will just define the same function, because debug.cc is a strange place to look for it. Or maybe that's not a big deal. What do you think? http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1782 src/debug.cc:1782: Handle<Code> lazy_compile( Handle<Code> lazy_compile = isolate_->builtins()->LazyCompile(); http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1813 src/debug.cc:1813: while (((obj = iterator.next()) != NULL)) { Maybe it doesn't matter, but should you avoid scanning map, code, and data spaces? http://codereview.chromium.org/8050010/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
