It doesn't need to have this logic.
ParseLazyFunctionLiteralBody is basically just ParseStatementList
+ log the function position. But PreParser doesn't need to have
the "which functions to log" logic, since logging the function is
always done exactly when Parser falls back to PreParser. (See
PreParseLazyFunction.)
So in the current state, PreParser would log several functions in
a SingletonLogger, and only the last one would take
effect (that's the one Parser also logs in SkipLazyFunctionBody).
Also updated test-parsing/Regress928 to produce the preparse data
the way we do now (i.e., not running the PreParser directly, but
running the Parser).
Error reporting: when PreParser finds an error, it doesn't need
to ReportUnexpectedToken in PreParseLazyFunction, since it
already has reported the error whenever it found it.
BUG=v8:5515
Review-Url: https://codereview.chromium.org/2421833002
Cr-Commit-Position: refs/heads/master@{#40315}
If an inner function only declares a variable but doesn't use it, Parser
and PreParser produced different unresolved variables, and that confused
the pessimistic context allocation.
This is continuation to https://codereview.chromium.org/2388183003/
This CL fixes more complicated declarations (which are not just one
identifier). For this, PreParser needs to accumulate identifiers used
in expressions.
In addition, this CL manifests FLAG_lazy_inner_functions in tests, so that
we get clusterfuzz coverage for it.
BUG=chromium:650969, v8:5501
Review-Url: https://codereview.chromium.org/2400613003
Cr-Commit-Position: refs/heads/master@{#40112}
If an inner function only declares a variable but doesn't use it, Parser
and PreParser produced different unresolved variables, and that confused
the pessimistic context allocation.
BUG=chromium:650969
Review-Url: https://codereview.chromium.org/2388183003
Cr-Commit-Position: refs/heads/master@{#39947}
Previously we'd have a scope in the main zone, and another in the temp zone. Then we carefully copied back data to the main zone. This CL changes it so that the scope is just fixed up to only contain data from the main zone. That avoids additional copies and additional allocations; while not increasing the care that needs to be taken. This will also make it easier to abort preparsing while parsing using a temp zone.
BUG=
Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
Review-Url: https://codereview.chromium.org/2368313002
Cr-Original-Commit-Position: refs/heads/master@{#39800}
Cr-Commit-Position: refs/heads/master@{#39828}
Reason for revert:
Revert due to asm.js slowdown
Original issue's description:
> Don't use different function scopes when parsing with temp zones
>
> Previously we'd have a scope in the main zone, and another in the temp zone. Then we carefully copied back data to the main zone. This CL changes it so that the scope is just fixed up to only contain data from the main zone. That avoids additional copies and additional allocations; while not increasing the care that needs to be taken. This will also make it easier to abort preparsing while parsing using a temp zone.
>
> BUG=
>
> Committed: https://crrev.com/f41e7ebd62b32e861b6aa14ad8bfce3018d03c3c
> Cr-Commit-Position: refs/heads/master@{#39800}
TBR=marja@chromium.org,adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review-Url: https://codereview.chromium.org/2379533003
Cr-Commit-Position: refs/heads/master@{#39821}
Previously we'd have a scope in the main zone, and another in the temp zone. Then we carefully copied back data to the main zone. This CL changes it so that the scope is just fixed up to only contain data from the main zone. That avoids additional copies and additional allocations; while not increasing the care that needs to be taken. This will also make it easier to abort preparsing while parsing using a temp zone.
BUG=
Review-Url: https://codereview.chromium.org/2368313002
Cr-Commit-Position: refs/heads/master@{#39800}
This patch moves the following parsing method to ParserBase:
- DesugarAsyncFunctionBody, renamed to ParseAsyncFunctionBody
- ParseAsyncFunctionExpression, renamed to ParseAsyncFunctionLiteral
- ParseAsyncFunctionDeclaration
It renames the parser implementation methods:
- ParseArrowFunctionFormalParameterList -> DeclareArrowFunctionFormalParameters
- ParseArrowFunctionFormalParameters -> AddArrowFunctionFormalParameters
It also eliminates method ParseAsyncArrowSingleExpressionBody.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2372733002
Cr-Commit-Position: refs/heads/master@{#39788}
Reason for revert:
Stability thief found, relanding speculative reverts.
Original issue's description:
> Revert of Preparse functions in the scope that was created when parsing of the function was started (patchset #2 id:20001 of https://codereview.chromium.org/2370713003/ )
>
> Reason for revert:
> Needed for https://codereview.chromium.org/2373443003/
>
> Original issue's description:
> > Preparse functions in the scope that was created when parsing of the function was started
> >
> > This reduces the number of scopes for lazily parsed top-level functions from 3 to 1
> >
> > BUG=v8:5209
> >
> > Committed: https://crrev.com/9618d095903c604a032b33792c068f4a6169503c
> > Cr-Commit-Position: refs/heads/master@{#39725}
>
> TBR=marja@chromium.org,verwaest@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=v8:5209
>
> Committed: https://crrev.com/0cef7100da0b609403c9026fb7307192a898a390
> Cr-Commit-Position: refs/heads/master@{#39729}
TBR=marja@chromium.org,verwaest@chromium.org,hablich@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5209
Review-Url: https://codereview.chromium.org/2377593002
Cr-Commit-Position: refs/heads/master@{#39756}
Reason for revert:
Stability thief found, relanding speculative reverts.
Original issue's description:
> Revert of Preparse inner functions (new try) (patchset #21 id:420001 of https://codereview.chromium.org/2352593002/ )
>
> Reason for revert:
> We currently have some stability issues on Canary. Let's reland this after we verified that we "fixed" Canary again.
>
> Original issue's description:
> > Preparse inner functions (new try)
> >
> > This is an overly pessimistic approach where PreParser only keeps
> > track of unresolved variables, but doesn't declare anything. This
> > will result in context-allocating variables in the outer function
> > unnecessarily, if the variable names clash with variable names
> > used by the inner function (even if the variables are not the
> > same). However, we have been unable to prove that this approach
> > wouldn't be good enough for the practical purposes.
> >
> > Fixes after the previous try ( https://codereview.chromium.org/2322243002/ ):
> > Keep the context-allocation decision stable when compiling fully eagerly.
> >
> > Tests which exercise this functionality:
> > mjsunit/fixed-context-shapes-when-recompiling.js
> >
> > Design document (chromium):
> >
> > https://docs.google.com/a/chromium.org/document/d/1rRv5JJZ0JpOZAZN2CSUwZPFJiBAdRnTiSYhazseNHFg/edit?usp=sharing
> >
> > BUG=
> >
> > Committed: https://crrev.com/7c73cf32c60484cdf37c84f1d61b4640e87068d7
> > Cr-Commit-Position: refs/heads/master@{#39719}
>
> TBR=verwaest@chromium.org,adamk@chromium.org,marja@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=
>
> Committed: https://crrev.com/1e6296b2a7cfc307fd9e722e619f42965da4a267
> Cr-Commit-Position: refs/heads/master@{#39730}
TBR=verwaest@chromium.org,adamk@chromium.org,marja@chromium.org,hablich@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review-Url: https://codereview.chromium.org/2377513006
Cr-Commit-Position: refs/heads/master@{#39755}
Reason for revert:
We currently have some stability issues on Canary. Let's reland this after we verified that we "fixed" Canary again.
Original issue's description:
> Preparse inner functions (new try)
>
> This is an overly pessimistic approach where PreParser only keeps
> track of unresolved variables, but doesn't declare anything. This
> will result in context-allocating variables in the outer function
> unnecessarily, if the variable names clash with variable names
> used by the inner function (even if the variables are not the
> same). However, we have been unable to prove that this approach
> wouldn't be good enough for the practical purposes.
>
> Fixes after the previous try ( https://codereview.chromium.org/2322243002/ ):
> Keep the context-allocation decision stable when compiling fully eagerly.
>
> Tests which exercise this functionality:
> mjsunit/fixed-context-shapes-when-recompiling.js
>
> Design document (chromium):
>
> https://docs.google.com/a/chromium.org/document/d/1rRv5JJZ0JpOZAZN2CSUwZPFJiBAdRnTiSYhazseNHFg/edit?usp=sharing
>
> BUG=
>
> Committed: https://crrev.com/7c73cf32c60484cdf37c84f1d61b4640e87068d7
> Cr-Commit-Position: refs/heads/master@{#39719}
TBR=verwaest@chromium.org,adamk@chromium.org,marja@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review-Url: https://codereview.chromium.org/2373443003
Cr-Commit-Position: refs/heads/master@{#39730}
Reason for revert:
Needed for https://codereview.chromium.org/2373443003/
Original issue's description:
> Preparse functions in the scope that was created when parsing of the function was started
>
> This reduces the number of scopes for lazily parsed top-level functions from 3 to 1
>
> BUG=v8:5209
>
> Committed: https://crrev.com/9618d095903c604a032b33792c068f4a6169503c
> Cr-Commit-Position: refs/heads/master@{#39725}
TBR=marja@chromium.org,verwaest@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5209
Review-Url: https://codereview.chromium.org/2365393002
Cr-Commit-Position: refs/heads/master@{#39729}
This reduces the number of scopes for lazily parsed top-level functions from 3 to 1
BUG=v8:5209
Review-Url: https://codereview.chromium.org/2370713003
Cr-Commit-Position: refs/heads/master@{#39725}
This is an overly pessimistic approach where PreParser only keeps
track of unresolved variables, but doesn't declare anything. This
will result in context-allocating variables in the outer function
unnecessarily, if the variable names clash with variable names
used by the inner function (even if the variables are not the
same). However, we have been unable to prove that this approach
wouldn't be good enough for the practical purposes.
Fixes after the previous try ( https://codereview.chromium.org/2322243002/ ):
Keep the context-allocation decision stable when compiling fully eagerly.
Tests which exercise this functionality:
mjsunit/fixed-context-shapes-when-recompiling.js
Design document (chromium):
https://docs.google.com/a/chromium.org/document/d/1rRv5JJZ0JpOZAZN2CSUwZPFJiBAdRnTiSYhazseNHFg/edit?usp=sharing
BUG=
Review-Url: https://codereview.chromium.org/2352593002
Cr-Commit-Position: refs/heads/master@{#39719}
Reason for revert:
This approach is not good - breaks when we recompile.
Original issue's description:
> Preparse inner functions.
>
> This is an overly pessimistic approach where PreParser only keeps
> track of unresolved variables, but doesn't declare anything. This
> will result in context-allocating variables in the outer function
> unnecessarily, if the variable names clash with variable names
> used by the inner function (even if the variables are not the
> same). However, we have been unable to prove that this approach
> wouldn't be good enough for the practical purposes.
>
> Committed: https://crrev.com/e1341ca8fa486bb2c9e4236672a64ec7756a164d
> Cr-Commit-Position: refs/heads/master@{#39469}
TBR=adamk@chromium.org,vogelheim@chromium.org,nikolaos@chromium.org,nednguyen@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.chromium.org/2349473004
Cr-Commit-Position: refs/heads/master@{#39471}
This is an overly pessimistic approach where PreParser only keeps
track of unresolved variables, but doesn't declare anything. This
will result in context-allocating variables in the outer function
unnecessarily, if the variable names clash with variable names
used by the inner function (even if the variables are not the
same). However, we have been unable to prove that this approach
wouldn't be good enough for the practical purposes.
Review-Url: https://codereview.chromium.org/2322243002
Cr-Commit-Position: refs/heads/master@{#39469}
This patch moves the following parsing method to ParserBase:
- ParseSwitchStatement
It also removes ParseCaseClause and merges it with ParseSwitchStatement,
mainly to avoid the complexity of introducing one more abstract typedef
to be shared between parser implementations, but also because the merged
ParseSwitchStatement is now only 59 lines.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2324843005
Cr-Commit-Position: refs/heads/master@{#39337}
This patch moves the following parsing methods to ParserBase:
- ParseScopedStatement
- ParseVariableStatement
- ParseDebuggerStatement
- ParseV8Intrinsic
It also cleans up the implementation-specific use counter mechanism.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2318263002
Cr-Commit-Position: refs/heads/master@{#39272}
Move the code to perform function name inference for properties into
parsing the properties themselves, instead of the containing object.
This allows us to avoid unnecessary calls when parsing shorthand
properties and methods and simplifies the logic in the remaining cases.
Also fixes an edge case bug: inferring the name of the getter in
`class { static get constructor(){} }`.
Review-Url: https://codereview.chromium.org/2313723005
Cr-Commit-Position: refs/heads/master@{#39222}
This introduces ClassLiteralProperty and a supertype LiteralProperty of
it and ObjectLiteralProperty. It also splits the parsing of the two.
This substiantially clarifies some logic, especially as classes
continue to evolve, and is also about a 2% performance improvement to
parsing either kind of property (since no work is wasted on logic
only necessary for the other kind). Also, it saves a word on
ObjectLiteralProperties.
Review-Url: https://codereview.chromium.org/2302643002
Cr-Commit-Position: refs/heads/master@{#39219}
This patch moves the following parsing methods to ParserBase:
- ParseStatementList
- ParseStatementListItem
- ParseStatement
- ParseSubStatement (subsumed in ParseStatement)
- ParseStatementAsUnlabeled
It also refactors the Target and TargetScope objects, used by the
parser.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Committed: https://crrev.com/df29f3fda25660075a273cc27ad9f7787f321072
Review-Url: https://codereview.chromium.org/2307073002
Cr-Original-Commit-Position: refs/heads/master@{#39167}
Cr-Commit-Position: refs/heads/master@{#39175}
This patch moves the following parsing methods to ParserBase:
- ParseStatementList
- ParseStatementListItem
- ParseStatement
- ParseSubStatement (subsumed in ParseStatement)
- ParseStatementAsUnlabeled
It also refactors the Target and TargetScope objects, used by the
parser.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2307073002
Cr-Commit-Position: refs/heads/master@{#39167}
This enables PreParser to declare variables in the future without
duplicating the parsing logic.
BUG=
Review-Url: https://codereview.chromium.org/2297563007
Cr-Commit-Position: refs/heads/master@{#39079}
This patch refactors the scanner bookmark in SkipLazyFunctionBody,
so that it is only used locally, instead of being passed to several
other methods. It is replaced by a "may_abort" parameter and an
appropriate result denoting whether lazy parsing has been aborted.
It also applies the hack of aborting lazy parsing for arrow
functions that are considered to be "initialization functions".
R=adamk@chromium.org, vogelheim@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2297733002
Cr-Commit-Position: refs/heads/master@{#39072}
This patch removes the explicit classifier parameters from all
parsing methods and makes expression classifiers implicit in
the (pre)parser's implementation. In this way, the implementation
is simplified and a proper stack of classifiers is enforced.
R=adamk@chromium.org,littledan@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2289663002
Cr-Commit-Position: refs/heads/master@{#39068}
This patch arranges that property names are parsed in a single pass,
reporting the name as well as the type of the property, instead of
parsing qualifiers like 'static' or 'get' initially as names and then
re-parsing. This change is easier to reason about, very slightly (4%)
faster in some cases (although slower in other, less common ones, though
this slowdown will be fixed in an upcoming patch), and is a prerequisite
for separating the parsing of object and class literal properties, which
will become increasingly important as ECMAScript adds more class features.
This is a reland of https://codereview.chromium.org/2278153004/,
which fixes the issue causing the revert and adds more tests.
Review-Url: https://codereview.chromium.org/2300503002
Cr-Commit-Position: refs/heads/master@{#39056}
Reason for revert:
Fails to reject "{*foo: 1}" as an object literal, found
by the fuzzer:
https://build.chromium.org/p/client.v8/builders/V8%20Fuzzer/builds/12315/steps/Fuzz%20on%20Ubuntu-12.04/logs/stdio
Original issue's description:
> Refactor object/class literal property name parsing
>
> This patch arranges that property names are parsed in a single pass,
> reporting the name as well as the type of the property, instead of
> parsing qualifiers like 'static' or 'get' initially as names and then
> re-parsing. This change is easier to reason about, very slightly (4%)
> faster in some cases (although slower in other, less common ones, though
> this slowdown will be fixed in an upcoming patch), and is a prerequisite
> for separating the parsing of object and class literal properties, which
> will become increasingly important as ECMAScript adds more class features.
>
> Committed: https://crrev.com/6dd26c729584024e17a05a2a76b319d4aecdc138
> Cr-Commit-Position: refs/heads/master@{#39027}
TBR=littledan@chromium.org,marja@chromium.org,bakkot@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.chromium.org/2295743003
Cr-Commit-Position: refs/heads/master@{#39029}
This patch arranges that property names are parsed in a single pass,
reporting the name as well as the type of the property, instead of
parsing qualifiers like 'static' or 'get' initially as names and then
re-parsing. This change is easier to reason about, very slightly (4%)
faster in some cases (although slower in other, less common ones, though
this slowdown will be fixed in an upcoming patch), and is a prerequisite
for separating the parsing of object and class literal properties, which
will become increasingly important as ECMAScript adds more class features.
Review-Url: https://codereview.chromium.org/2278153004
Cr-Commit-Position: refs/heads/master@{#39027}
Previously the calls to ExpressionClassifier::Accumulate() each chose
slightly different sets of productions to accumulate, and it turned
out that these were in some cases broader than needed and in some
cases less broad.
The existence of some grab-bag production bitmasks like
ExpressionClassifier::ExpressionProductions made this situation more
error-prone (for example, that production was missing AsyncArrowFormalParametersProduction).
This patch removes all "grab-bags" besides AllProductions. In some of
the places where code was using those grab-bags for convenience, it
switches them to use negation of AllProductions. In other, specifically
those having to do with expressions that are disallowed anywhere in
a sub-expression of a parameter list, I've added a new method on
ExpressionClassifier to centralize the logic.
The aforementioned centralization/addition of
AsyncArrowFormalParametersProduction fixes several cases where we were
failing to report an error for 'await' in some contexts; I've added
those test cases.
The patch also narrows all cases to exactly the set or productions
necessary, with a comment on each explaining the choice.
BUG=v8:4483
Review-Url: https://codereview.chromium.org/2271063002
Cr-Commit-Position: refs/heads/master@{#38918}
DuplicateFinder isn't actually used by the Scanner, except for one
convenience function which we should probably remove, also.
BUG=
Review-Url: https://codereview.chromium.org/2281443002
Cr-Commit-Position: refs/heads/master@{#38904}
This patch moves the following methods from the traits objects to
the (pre)parser implementation objects:
- ExpressionFromIdentifier
- ExpressionFromLiteral
- ExpressionFromString
- FunctionSentExpression
- GetNextSymbol
- GetNumberAsSymbol
- GetSymbol
- NewExpressionList
- NewPropertyList
- NewStatementList
- NewSuperCallReference
- NewSuperPropertyReference
- NewTargetExpression
- ThisExpression
Also, the method GetIterator is specific only to the parser and is
removed from the preparser's implementation.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2274113002
Cr-Commit-Position: refs/heads/master@{#38890}
This patch removes 26 elements of the (pre)parser traits objects.
Some methods are removed completely and called directly from the
implementation objects:
- ParseAsyncFunctionExpression
- ParseClassLiteral
- ParseDoExpression
- ParseEagerFunctionBody
- ParseFunctionLiteral
- ParseV8Intrinsic
Some methods have to be moved to at least one implementation object:
- AddTemplateExpression
- AddTemplateSpan
- CheckConflictingVarDeclarations
- CloseTemplateLiteral
- MarkCollectedTailCallExpressions
- MarkTailPosition
- OpenTemplateLiteral
- ParseAsyncArrowSingleExpressionBody
- PrepareSpreadArguments
- QueueDestructuringAssignmentForRewriting
- QueueNonPatternForRewriting
- RewriteAssignExponentiation
- RewriteAwaitExpression
- RewriteDestructuringAssignments
- RewriteExponentiation
- RewriteNonPattern
- RewriteYieldStar
- SkipLazyFunctionBody
- SpreadCall
- SpreadCallNew
Also, the inner class/struct TemplateLiteralState is moved to the
implementation objects.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2267783002
Cr-Commit-Position: refs/heads/master@{#38837}
This flag was only set on receiver scopes (declaration scopes) already. This makes it statically obvious.
BUG=v8:5209
Review-Url: https://codereview.chromium.org/2268333002
Cr-Commit-Position: refs/heads/master@{#38828}
This patch applies an adaptation of the Curiously Recurring Template
Pattern to the parser objects. The result is roughly:
// Common denominator, needed to avoid cyclic dependency.
// Instances of this template will end up with very minimal
// definitions, ideally containing just typedefs.
template <typename Impl>
class ParserBaseTraits;
// The parser base object, which should just implement pure
// parser behavior. The Impl parameter is the actual derived
// class (according to CRTP), which implements impure parser
// behavior.
template <typename Impl>
class ParserBase : public ParserBaseTraits<Impl> { ... };
// And then, for each parser variant:
class Parser;
template <>
class ParserBaseTraits<Parser> { ... };
class Parser : public ParserBase<Parser> { ... };
Using the CRTP, we will ultimately achieve two goals:
(1) clean up the traits objects, but most importantly
(2) clearly separate pure/impure parser implementation and facilitate
experimentation with different parser variants.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2267663002
Cr-Commit-Position: refs/heads/master@{#38819}
parser and the preparser, so that they contain the same set of methods,
with the same signatures. It mainly flags some traits methods as const.
It also contains a small cosmetic change in the definition of CHECK_OK.
R=adamk@chromium.org, marja@chromium.org
BUG=
LOG=N
Review-Url: https://codereview.chromium.org/2258123002
Cr-Commit-Position: refs/heads/master@{#38767}
