On 2014/06/18 14:54:14, aperez wrote:
On 2014/06/17 11:47:38, marja wrote:
https://codereview.chromium.org/160073006/diff/500001/src/parser.cc#newcode3323
> src/parser.cc:3323: error_token = "";
> In this case, don't we get an incorrect error end position?
ParamListFinder in the current implementation will give this error instead
(that code path should not be reached):
(d8):1: SyntaxError: Function parameter list is malformed
(32, a) => {}
^^^^^^
SyntaxError: Function parameter list is malformed
Unfortunately, the error does not point at the exact point where the
issue is.
I think that's a good enough error.
https://codereview.chromium.org/160073006/diff/500001/src/scanner.cc#newcode935
> src/scanner.cc:935: bool Scanner::IdentifierIsFutureStrictReserved(const
> AstString* string) const {
> This sounds wrong, since we have a separate token for
FUTURE_RESERVED_WORD,
so
> here we have somehow lost that information and we're trying to get it
back..
:/
We are losing the information when lifting things up to the AST or
to the PreParserIdentifier/PreParserExpression objects. Would it be
acceptable to keep in the Scanner the positions of the last seen
future reserved word, last "eval" token, etc? That would be enough
(I think) for detecting those as part of a parameter list in strict
mode.
I'm not at all buying this "add code in the Scanner to enhance information
that
PreParser lost" approach; instead, we should not lose the information to
begin
with. Or if that's not feasible, then PreParser should just not produce
errors
for those things (but that's not a very popular view either).
The problem here is that PreParser
(PreParserIdentifier/PreParserExpression)
does not tracks enough information: a single PreParserExpression is
returned
from ParseAssignmentExpression(), while in Parser it returns an AST
subtree
of binops/variableproxies. So I would need it to actually return a list of
identifiers seen (or something equivalent), their positions,
parenthesization,
and for them to have associated strings with the identifier names -- not
just
an integer value with a set of bit flags.
Wdyt of the idea that PreParserExpression would track the "is comma
separated
identifier list" property?
Like, if we see "foo", that's a PreParserExpression that is an identifier
(we
can track that), the same for "bar", and when we build the comma expression
"foo, bar", we can track that the corresponding comma expression is a comma
separated identifier list (and furthermore for "foo, bar, baz" (further
comma
operation), and the parens, and that should do the trick.
But like you said, PreParser will get closer to Parser as we are required to
produce more early errors, and at some point, if we really cannot do without
AST, we need to kill it :(
https://codereview.chromium.org/160073006/
--
--
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.
