Reviewers: marja, rossberg,
https://codereview.chromium.org/974213002/diff/1/src/background-parsing-task.cc
File src/background-parsing-task.cc (right):
https://codereview.chromium.org/974213002/diff/1/src/background-parsing-task.cc#newcode25
src/background-parsing-task.cc:25:
info->set_source_stream(source->source_stream.get());
On 2015/03/04 09:24:40, marja wrote:
Would it make sense to have a ctor of ParseInfo which takes these
things as
params?
I tried that, and you end up with ctor hell with default-initializing
all the fields three different times in three different constructors. I
changed the InitFrom* functions to return this, to allow chaining.
https://codereview.chromium.org/974213002/diff/1/src/background-parsing-task.cc#newcode34
src/background-parsing-task.cc:34: options =
ScriptCompiler::kNoCompileOptions;
On 2015/03/04 09:24:40, marja wrote:
Huh, the options & options_ modifying is a bit unclear and hard to see
that it's
going correctly now.
Ok, I've fixed this so the options are only stored in the ParseInfo now.
https://codereview.chromium.org/974213002/diff/1/src/background-parsing-task.cc#newcode38
src/background-parsing-task.cc:38: options_ =
ScriptCompiler::kNoCompileOptions;
On 2015/03/04 09:24:40, marja wrote:
... why is this correct? Won't this just always kill the compile
options? (and
we'll never get into the source_->info->set_cached_data(&script_data);
branch in
Run).
Some tests *should* fail if you mess this up, though :)
Fixed.
https://codereview.chromium.org/974213002/diff/1/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/974213002/diff/1/src/compiler.cc#newcode171
src/compiler.cc:171: if (parse_info_)
parse_info_->DeleteAstValueFactory();
On 2015/03/04 09:24:40, marja wrote:
... I guess parse_info_ is not owned, CompilationInfo just happens to
point into
it... so it's slightly surprising that the lifetime of CompilationInfo
affects
what happens inside ParseInfo.
Oh yeah, good point, probably a bug. CompilationInfoWithZone owns the
ParseInfo and tears it down in the virtual destructor, but a "naked"
CompilationInfo doesn't own the ParseInfo and should probably not try to
mess with its internal AstValueFactory.
https://codereview.chromium.org/974213002/diff/1/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/974213002/diff/1/src/parser.h#newcode27
src/parser.h:27: class ScriptData {
On 2015/03/04 09:24:40, marja wrote:
Can you put the added classes (ScriptData and ParseInfo) into a
separate file?
parser.h is bloated enough already and diff-wise it's the same.
ScriptData probably should live somewhere else, but I'm on the fence
about ParseInfo.
https://codereview.chromium.org/974213002/diff/1/src/parser.h#newcode75
src/parser.h:75: enum Flag {
On 2015/03/04 10:25:12, rossberg wrote:
Can we make the enum private?
Done.
Description:
Extract ParseInfo from CompilationInfo.
Rationale: separate the inputs and outputs of parsing + analysis from the
business of compiling (i.e. generating machine code).
BUG=
Please review this at https://codereview.chromium.org/974213002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+668, -566 lines):
M src/api.cc
M src/ast-numbering.cc
M src/background-parsing-task.h
M src/background-parsing-task.cc
M src/compiler.h
M src/compiler.cc
M src/compiler/ast-loop-assignment-analyzer.cc
M src/compiler/basic-block-instrumentor.cc
M src/compiler/js-inlining.cc
M src/compiler/linkage.cc
M src/globals.h
M src/hydrogen.cc
M src/parser.h
M src/parser.cc
M src/preparse-data.cc
M src/rewriter.h
M src/rewriter.cc
M src/runtime/runtime-debug.cc
M src/runtime/runtime-internal.cc
M src/scopes.h
M src/scopes.cc
M src/serialize.cc
M test/cctest/compiler/function-tester.h
M test/cctest/compiler/test-codegen-deopt.cc
M test/cctest/compiler/test-loop-assignment-analysis.cc
M test/cctest/compiler/test-pipeline.cc
M test/cctest/test-parsing.cc
M test/cctest/test-serialize.cc
M tools/parser-shell.cc
--
--
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.
