I had a look at the following parts: parser, compiler, scopes,
background-parsing-task, tests, parser-shell. They lgtm modulo the comments
(afaics it's leaking Zone). (The other parts look straightforward too,
can't see
anything wrong there either, you can decide whether they're non-trivial
enough
to ask somebody else to review too.)
https://codereview.chromium.org/974213002/diff/120001/src/background-parsing-task.cc
File src/background-parsing-task.cc (left):
https://codereview.chromium.org/974213002/diff/120001/src/background-parsing-task.cc#oldcode20
src/background-parsing-task.cc:20: // We don't set the context to the
CompilationInfo yet, because the background
This comment is probably still relevant; why was it removed?
https://codereview.chromium.org/974213002/diff/120001/src/background-parsing-task.cc
File src/background-parsing-task.cc (right):
https://codereview.chromium.org/974213002/diff/120001/src/background-parsing-task.cc#newcode20
src/background-parsing-task.cc:20: Zone* zone = new Zone();
Who deletes Zone? ParseInfo doesn't own it afaics.
https://codereview.chromium.org/974213002/diff/120001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/974213002/diff/120001/src/compiler.cc#newcode1099
src/compiler.cc:1099: info->parse_info()->set_cached_data(nullptr);
Nit: you already have parse_info, so s/info->parse_info()/parse_info/
https://codereview.chromium.org/974213002/diff/120001/src/compiler.cc#newcode1593
src/compiler.cc:1593: if (parse_info_) {
Nit: unnecessary if, since delete 0 is ok.
https://codereview.chromium.org/974213002/
--
--
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.
