https://codereview.chromium.org/974213002/diff/1/src/globals.h
File src/globals.h (right):
https://codereview.chromium.org/974213002/diff/1/src/globals.h#newcode457
src/globals.h:457: enum ParseRestriction {
On 2015/03/04 10:25:12, rossberg wrote:
I don't think this belongs into global.h - it encodes a choice of
start symbol
for parsing. In that sense, it has a similar role to flags like
parse_info->is_module(), and should probably be handled analogously.
I.e., just
kill the enum and have a is_function flag in parse info. (And later we
should
probably unify these flags into one grammar_entry kind of value.)
Short of that, can you at least move it to parser.h?
Michi argued strongly to keep the enum.
I initially tried to move it to parser.h, but then you end up with a
cycle between compiler.h and parser.h. Actually this cycle already
exists and it needs to be broken carefully to reduce the over-inclusion
of compiler.h. Couldn't figure out how to do that yet.
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 22:39:35, titzer wrote:
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.
I've moved ScriptData to preparse-data.h
https://codereview.chromium.org/974213002/diff/1/src/parser.h#newcode228
src/parser.h:228: // TODO(titzer): these should *not* be part of
ParseInfo. Refactor Parser.
On 2015/03/04 09:24:40, marja wrote:
Yes! :)
I'm not sure exactly where these will land, but I was thinking of
further splitting out another small struct with just these handles.
https://codereview.chromium.org/974213002/diff/80001/src/background-parsing-task.cc
File src/background-parsing-task.cc (right):
https://codereview.chromium.org/974213002/diff/80001/src/background-parsing-task.cc#newcode31
src/background-parsing-task.cc:31: bool disable_lazy =
!Compiler::DebuggerWantsEagerCompilation(isolate);
On 2015/03/05 10:52:55, marja wrote:
... shouldn't we disable_lazy exactly when
DebuggerWantsEagerCompilation,
without negation?
Good catch, thankfully the test failed :-)
https://codereview.chromium.org/974213002/diff/80001/src/background-parsing-task.cc#newcode39
src/background-parsing-task.cc:39: source->allow_lazy = !disable_lazy;
On 2015/03/05 10:52:55, marja wrote:
... and why do we need allow_lazy now that it's already known by info?
Good point, I've removed both allow_lazy and hash_seed.
Also, I've removed the boolean allow_lazy from Parser::ParseStatic,
since it is now contained in the ParseInfo.
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.
