I like this (i.e., it's very close to how I would have done it).
The problems with detecting "use strict" can be fixed without resorting to
extra
passes.
The rest looks well structured and follows the existing conventions well.
http://codereview.chromium.org/6144005/diff/1/src/parser.cc
File
http://codereview.chromium.org/6144005/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode1084
src/parser.cc:1084: if (peek() == Token::STRING) {
Argh, location should be:
Scanner::Location location = scanner().peek_location();
a
After some thought, we think it will be a good idea to have a flag that can
be
used to disable strict mode detection.
It should be enabled by default, but the flag will let us test whether it's
strict-mode that makes something fail.
It should be sufficient to check it only at the point where we
Just to clearify. Having a flag to disable which is enabled by default is a
bit confusing. ;) We want strict-mode support on by default! The flag can be
used to test if pages break because of strict mode or something else.
Thanks!-- Mads
On Thu, Jan 13, 2011 at 11:29 AM, wrote:
> After some
FYI, here's an instance of needing to disable strict mode:
https://bugs.webkit.org/show_bug.cgi?id=48377
http://codereview.chromium.org/6144005/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
Thanks for the feedback, Lasse!
Tried to incorporate all of it, in few places I don't know how nervous to be
about additional work now happening when processing non-strict code.
Also adding --strict_mode flag.
Main question - how does one allocate transient memory in the parser? A
memory
that
http://codereview.chromium.org/6144005/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode1084
src/parser.cc:1084: if (peek() == Token::STRING) {
I don't see how that example applies. The "Hello" String is a string
literal. When pa
Thank you for reviewing the code for me!
Here it is updated per your feedback.
Martin
http://codereview.chromium.org/6144005/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode1084
src/parser.cc:1084: if (peek() == Token::STRING)
LGTM.
http://codereview.chromium.org/6144005/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode1084
src/parser.cc:1084: if (peek() == Token::STRING) {
Ack, yes, now I see.
http://codereview.chromium.org/6144005/
--
v8-dev mail
http://codereview.chromium.org/6144005/diff/20001/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/20001/src/parser.cc#newcode1076
src/parser.cc:1076: if (directive_prologue && peek() == Token::STRING) {
So I guess this could just be
if (directive_prologue &
Made the change and added tests to cover the currently implemented strict
mode
checks.
http://codereview.chromium.org/6144005/diff/20001/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6144005/diff/20001/src/parser.cc#newcode1076
src/parser.cc:1076: if (directive_prol
LGTM.
Let's try to commit it and see the impact!
http://codereview.chromium.org/6144005/diff/32001/src/messages.js
File src/messages.js (right):
http://codereview.chromium.org/6144005/diff/32001/src/messages.js#newcode205
src/messages.js:205: strict_function_name: "Function name may
not
Thanks, fixed these issues, added couple more tests and committed.
Thank you!
Martin
http://codereview.chromium.org/6144005/diff/32001/src/messages.js
File src/messages.js (right):
http://codereview.chromium.org/6144005/diff/32001/src/messages.js#newcode205
src/messages.js:205: strict_function
The commit with n^2 algorithm for parameter validation caused timeout in
Mozilla
tests (function with 65536 parameters, amounting to ~4G pairs to check).
I am adding back the hashmap based checking. During parsing of parameters,
all
locations are stored in the list and later parameters are ch
Methinks parameter validation doesn't belong in the parser. After parsing
we
resolve variables in scopes.cc, which is already hashing and canonicalizing
variable references. Wouldn't that be the right place to check all things
scope-related?
We should avoid adding extra passes and auxiliary d
It would make sense, as long as we can emit a SyntaxError with the correct
location at that time.
ES5 requires an "early error" for these syntax errors, but any time before
running any code should be fine.
We will need resolved variables anyway, in order to emit early SyntaxError
for deleting local
That should be doable. The code for const redeclarations is an example
(though we report the error later, I think we have all the right source info
in it and could as well report it when we discover it).
On Wed, Jan 19, 2011 at 10:21 AM, Lasse R.H. Nielsen wrote:
> It would make sense, as long a
Doh, not , SyntaxError for delete is not early.
/L
On Wed, Jan 19, 2011 at 10:21, Lasse R.H. Nielsen wrote:
> It would make sense, as long as we can emit a SyntaxError with the correct
> location at that time.
> ES5 requires an "early error" for these syntax errors, but any time before
> running
Thanks,
It makes sense to minimize validation in the parser, but as Lasse points
out, we drop the location info.
As for this commit, I'd like to remove the parameter validation for now and
commit without it.
Then, I'd explore the delayed parameter validation which can mean:
1) good errors to the
Removing the parameter validation for now, will find the right approach in
another patch.
Thank you!
Martin
http://codereview.chromium.org/6144005/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
So, since flags can be true by default, rather than a --disable-strict-mode
flag, which is false by default, we should
just have a --strict-mode flag, which is true by default. Right? Would
there also be a --always-strict-mode flag?
On Thu, Jan 13, 2011 at 11:32 AM, Mads Sig Ager wrote:
> Ju
Yes, that is what I meant to say :)
A "strict-mode" flag that defaults to true, and if it is false, ignore
attempts to set strict mode when seeing "use strict".
I don't think it makes much sense to have an "always-strict-mode" flag. The
expected behavior of code not written to be strict that is in
22 matches
Mail list logo
