 Chromium Code Reviews
 Chromium Code Reviews Issue 2160593002:
  Introduce parent ScopeState class and track the scope through the state in the parser  (Closed) 
  Base URL: https://chromium.googlesource.com/v8/v8.git@master
    
  
    Issue 2160593002:
  Introduce parent ScopeState class and track the scope through the state in the parser  (Closed) 
  Base URL: https://chromium.googlesource.com/v8/v8.git@master| Index: src/parsing/parser-base.h | 
| diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h | 
| index 4a61f0726800c31c1a90eb07bb15e68186bc8422..6a8957577d8b4916557edb5c3febfd3f4878018a 100644 | 
| --- a/src/parsing/parser-base.h | 
| +++ b/src/parsing/parser-base.h | 
| @@ -179,7 +179,7 @@ class ParserBase : public Traits { | 
| v8::Extension* extension, AstValueFactory* ast_value_factory, | 
| ParserRecorder* log, typename Traits::Type::Parser this_object) | 
| : Traits(this_object), | 
| - scope_(NULL), | 
| + state_(NULL), | 
| function_state_(NULL), | 
| extension_(extension), | 
| fni_(NULL), | 
| @@ -254,17 +254,29 @@ class ParserBase : public Traits { | 
| // The parser's current scope is in scope_. BlockState and FunctionState | 
| // constructors push on the scope stack and the destructors pop. They are also | 
| // used to hold the parser's per-function and per-block state. | 
| - class BlockState BASE_EMBEDDED { | 
| + class ScopeState BASE_EMBEDDED { | 
| public: | 
| - BlockState(Scope** scope_stack, Scope* scope) | 
| - : scope_stack_(scope_stack), outer_scope_(*scope_stack) { | 
| - *scope_stack_ = scope; | 
| + V8_INLINE Scope* scope() const { return scope_; } | 
| + | 
| + protected: | 
| + ScopeState(ScopeState** scope_stack, Scope* scope) | 
| + : scope_stack_(scope_stack), outer_scope_(*scope_stack), scope_(scope) { | 
| + *scope_stack = this; | 
| } | 
| - ~BlockState() { *scope_stack_ = outer_scope_; } | 
| + ~ScopeState() { *scope_stack_ = outer_scope_; } | 
| + | 
| + Zone* zone() const { return scope_->zone(); } | 
| private: | 
| - Scope** scope_stack_; | 
| - Scope* outer_scope_; | 
| + ScopeState** scope_stack_; | 
| + ScopeState* outer_scope_; | 
| + Scope* scope_; | 
| + }; | 
| + | 
| + class BlockState final : public ScopeState { | 
| + public: | 
| + BlockState(ScopeState** scope_stack, Scope* scope) | 
| + : ScopeState(scope_stack, scope) {} | 
| }; | 
| struct DestructuringAssignment { | 
| @@ -334,10 +346,10 @@ class ParserBase : public Traits { | 
| kInsideForInOfBody, | 
| }; | 
| - class FunctionState BASE_EMBEDDED { | 
| + class FunctionState final : public ScopeState { | 
| public: | 
| - FunctionState(FunctionState** function_state_stack, Scope** scope_stack, | 
| - Scope* scope, FunctionKind kind, | 
| + FunctionState(FunctionState** function_state_stack, | 
| + ScopeState** scope_stack, Scope* scope, FunctionKind kind, | 
| typename Traits::Type::Factory* factory); | 
| ~FunctionState(); | 
| @@ -436,13 +448,11 @@ class ParserBase : public Traits { | 
| private: | 
| void AddDestructuringAssignment(DestructuringAssignment pair) { | 
| - destructuring_assignments_to_rewrite_.Add(pair, (*scope_stack_)->zone()); | 
| + destructuring_assignments_to_rewrite_.Add(pair, this->zone()); | 
| } | 
| - V8_INLINE Scope* scope() { return *scope_stack_; } | 
| - | 
| void AddNonPatternForRewriting(ExpressionT expr, bool* ok) { | 
| - non_patterns_to_rewrite_.Add(expr, (*scope_stack_)->zone()); | 
| + non_patterns_to_rewrite_.Add(expr, this->zone()); | 
| if (non_patterns_to_rewrite_.length() >= | 
| std::numeric_limits<uint16_t>::max()) | 
| *ok = false; | 
| @@ -473,8 +483,6 @@ class ParserBase : public Traits { | 
| FunctionState** function_state_stack_; | 
| FunctionState* outer_function_state_; | 
| - Scope** scope_stack_; | 
| - Scope* outer_scope_; | 
| ZoneList<DestructuringAssignment> destructuring_assignments_to_rewrite_; | 
| TailCallExpressionList tail_call_expressions_; | 
| @@ -791,7 +799,7 @@ class ParserBase : public Traits { | 
| return function_state_->factory(); | 
| } | 
| - LanguageMode language_mode() { return scope_->language_mode(); } | 
| + LanguageMode language_mode() { return scope()->language_mode(); } | 
| bool is_generator() const { return function_state_->is_generator(); } | 
| bool is_async_function() const { | 
| return function_state_->is_async_function(); | 
| @@ -1167,7 +1175,10 @@ class ParserBase : public Traits { | 
| bool has_seen_constructor_; | 
| }; | 
| - Scope* scope_; // Scope stack. | 
| + inline ModuleDescriptor* module() const { return scope()->module(); } | 
| + inline Scope* scope() const { return state_->scope(); } | 
| 
adamk
2016/07/18 17:14:45
No need for these "inline"s, unless you want to V8
 | 
| + | 
| + ScopeState* state_; // Scope stack. | 
| 
adamk
2016/07/18 17:14:45
scope_state_ seems like the right name here, at le
 | 
| FunctionState* function_state_; // Function state stack. | 
| v8::Extension* extension_; | 
| FuncNameInferrer* fni_; | 
| @@ -1197,9 +1208,10 @@ class ParserBase : public Traits { | 
| template <class Traits> | 
| ParserBase<Traits>::FunctionState::FunctionState( | 
| - FunctionState** function_state_stack, Scope** scope_stack, Scope* scope, | 
| - FunctionKind kind, typename Traits::Type::Factory* factory) | 
| - : next_materialized_literal_index_(0), | 
| + FunctionState** function_state_stack, ScopeState** scope_stack, | 
| + Scope* scope, FunctionKind kind, typename Traits::Type::Factory* factory) | 
| + : ScopeState(scope_stack, scope), | 
| + next_materialized_literal_index_(0), | 
| expected_property_count_(0), | 
| this_location_(Scanner::Location::invalid()), | 
| return_location_(Scanner::Location::invalid()), | 
| @@ -1208,8 +1220,6 @@ ParserBase<Traits>::FunctionState::FunctionState( | 
| generator_object_variable_(NULL), | 
| function_state_stack_(function_state_stack), | 
| outer_function_state_(*function_state_stack), | 
| - scope_stack_(scope_stack), | 
| - outer_scope_(*scope_stack), | 
| destructuring_assignments_to_rewrite_(16, scope->zone()), | 
| tail_call_expressions_(scope->zone()), | 
| return_expr_context_(ReturnExprContext::kInsideValidBlock), | 
| @@ -1218,7 +1228,6 @@ ParserBase<Traits>::FunctionState::FunctionState( | 
| factory_(factory), | 
| next_function_is_parenthesized_(false), | 
| this_function_is_parenthesized_(false) { | 
| - *scope_stack_ = scope; | 
| *function_state_stack = this; | 
| if (outer_function_state_) { | 
| this_function_is_parenthesized_ = | 
| @@ -1230,7 +1239,6 @@ ParserBase<Traits>::FunctionState::FunctionState( | 
| template <class Traits> | 
| ParserBase<Traits>::FunctionState::~FunctionState() { | 
| - *scope_stack_ = outer_scope_; | 
| *function_state_stack_ = outer_function_state_; | 
| } | 
| @@ -1348,7 +1356,7 @@ ParserBase<Traits>::ParseAndClassifyIdentifier(ExpressionClassifier* classifier, | 
| } | 
| } | 
| if (this->IsArguments(name)) { | 
| - scope_->RecordArgumentsUsage(); | 
| + scope()->RecordArgumentsUsage(); | 
| classifier->RecordStrictModeFormalParameterError( | 
| scanner()->location(), MessageTemplate::kStrictEvalArguments); | 
| if (is_strict(language_mode())) { | 
| @@ -1416,7 +1424,7 @@ ParserBase<Traits>::ParseIdentifierOrStrictReservedWord( | 
| } | 
| IdentifierT name = this->GetSymbol(scanner()); | 
| - if (this->IsArguments(name)) scope_->RecordArgumentsUsage(); | 
| + if (this->IsArguments(name)) scope()->RecordArgumentsUsage(); | 
| return name; | 
| } | 
| @@ -1436,7 +1444,7 @@ ParserBase<Traits>::ParseIdentifierName(bool* ok) { | 
| } | 
| IdentifierT name = this->GetSymbol(scanner()); | 
| - if (this->IsArguments(name)) scope_->RecordArgumentsUsage(); | 
| + if (this->IsArguments(name)) scope()->RecordArgumentsUsage(); | 
| return name; | 
| } | 
| @@ -1507,7 +1515,7 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier, | 
| case Token::THIS: { | 
| BindingPatternUnexpectedToken(classifier); | 
| Consume(Token::THIS); | 
| - return this->ThisExpression(scope_, factory(), beg_pos); | 
| + return this->ThisExpression(scope(), factory(), beg_pos); | 
| } | 
| case Token::NULL_LITERAL: | 
| @@ -1540,7 +1548,7 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier, | 
| // Using eval or arguments in this context is OK even in strict mode. | 
| IdentifierT name = ParseAndClassifyIdentifier(classifier, CHECK_OK); | 
| return this->ExpressionFromIdentifier( | 
| - name, beg_pos, scanner()->location().end_pos, scope_, factory()); | 
| + name, beg_pos, scanner()->location().end_pos, scope(), factory()); | 
| } | 
| case Token::STRING: { | 
| @@ -1966,7 +1974,7 @@ ParserBase<Traits>::ParsePropertyDefinition( | 
| } | 
| } | 
| ExpressionT lhs = this->ExpressionFromIdentifier( | 
| - *name, next_beg_pos, next_end_pos, scope_, factory()); | 
| + *name, next_beg_pos, next_end_pos, scope(), factory()); | 
| CheckDestructuringElement(lhs, classifier, next_beg_pos, next_end_pos); | 
| if (peek() == Token::ASSIGN) { | 
| @@ -2273,7 +2281,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, | 
| IdentifierT name = | 
| ParseAndClassifyIdentifier(&arrow_formals_classifier, CHECK_OK); | 
| expression = this->ExpressionFromIdentifier( | 
| - name, position(), scanner()->location().end_pos, scope_, factory()); | 
| + name, position(), scanner()->location().end_pos, scope(), factory()); | 
| } | 
| if (peek() == Token::ARROW) { | 
| @@ -2289,13 +2297,13 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, | 
| ValidateFormalParameterInitializer(&arrow_formals_classifier, ok); | 
| Scanner::Location loc(lhs_beg_pos, scanner()->location().end_pos); | 
| - Scope* scope = this->NewScope(scope_, FUNCTION_SCOPE, | 
| + Scope* scope = this->NewScope(this->scope(), FUNCTION_SCOPE, | 
| is_async ? FunctionKind::kAsyncArrowFunction | 
| : FunctionKind::kArrowFunction); | 
| // Because the arrow's parameters were parsed in the outer scope, any | 
| // usage flags that might have been triggered there need to be copied | 
| // to the arrow scope. | 
| - scope_->PropagateUsageFlagsToScope(scope); | 
| + this->scope()->PropagateUsageFlagsToScope(scope); | 
| FormalParametersT parameters(scope); | 
| if (!arrow_formals_classifier.is_simple_parameter_list()) { | 
| scope->SetHasNonSimpleParameters(); | 
| @@ -2717,7 +2725,7 @@ ParserBase<Traits>::ParseUnaryExpression(ExpressionClassifier* classifier, | 
| MessageTemplate::kAwaitBindingIdentifier); | 
| return this->ExpressionFromIdentifier( | 
| - name, beg_pos, scanner()->location().end_pos, scope_, factory()); | 
| + name, beg_pos, scanner()->location().end_pos, scope(), factory()); | 
| } | 
| default: | 
| break; | 
| @@ -2862,7 +2870,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression( | 
| // no explicit receiver. | 
| // These calls are marked as potentially direct eval calls. Whether | 
| // they are actually direct calls to eval is determined at run time. | 
| - this->CheckPossibleEvalCall(result, scope_); | 
| + this->CheckPossibleEvalCall(result, scope()); | 
| bool is_super_call = result->IsSuperCallReference(); | 
| if (spread_pos.IsValid()) { | 
| @@ -2875,7 +2883,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression( | 
| // Explicit calls to the super constructor using super() perform an | 
| // implicit binding assignment to the 'this' variable. | 
| if (is_super_call) { | 
| - ExpressionT this_expr = this->ThisExpression(scope_, factory(), pos); | 
| + ExpressionT this_expr = this->ThisExpression(scope(), factory(), pos); | 
| result = | 
| factory()->NewAssignment(Token::INIT, this_expr, result, pos); | 
| } | 
| @@ -3013,7 +3021,7 @@ ParserBase<Traits>::ParseMemberExpression(ExpressionClassifier* classifier, | 
| return this->EmptyExpression(); | 
| } | 
| - return this->FunctionSentExpression(scope_, factory(), pos); | 
| + return this->FunctionSentExpression(scope(), factory(), pos); | 
| } | 
| bool is_generator = Check(Token::MUL); | 
| @@ -3056,13 +3064,13 @@ ParserBase<Traits>::ParseSuperExpression(bool is_new, | 
| Expect(Token::SUPER, CHECK_OK); | 
| int pos = position(); | 
| - Scope* scope = scope_->ReceiverScope(); | 
| + Scope* scope = this->scope()->ReceiverScope(); | 
| FunctionKind kind = scope->function_kind(); | 
| if (IsConciseMethod(kind) || IsAccessorFunction(kind) || | 
| IsClassConstructor(kind)) { | 
| if (peek() == Token::PERIOD || peek() == Token::LBRACK) { | 
| scope->RecordSuperPropertyUsage(); | 
| - return this->NewSuperPropertyReference(scope_, factory(), pos); | 
| + return this->NewSuperPropertyReference(this->scope(), factory(), pos); | 
| } | 
| // new super() is never allowed. | 
| // super() is only allowed in derived constructor | 
| @@ -3070,7 +3078,7 @@ ParserBase<Traits>::ParseSuperExpression(bool is_new, | 
| // TODO(rossberg): This might not be the correct FunctionState for the | 
| // method here. | 
| function_state_->set_super_location(scanner()->location()); | 
| - return this->NewSuperCallReference(scope_, factory(), pos); | 
| + return this->NewSuperCallReference(this->scope(), factory(), pos); | 
| } | 
| } | 
| @@ -3100,14 +3108,14 @@ ParserBase<Traits>::ParseNewTargetExpression(bool* ok) { | 
| int pos = position(); | 
| ExpectMetaProperty(CStrVector("target"), "new.target", pos, CHECK_OK); | 
| - if (!scope_->ReceiverScope()->is_function_scope()) { | 
| + if (!scope()->ReceiverScope()->is_function_scope()) { | 
| ReportMessageAt(scanner()->location(), | 
| MessageTemplate::kUnexpectedNewTarget); | 
| *ok = false; | 
| return this->EmptyExpression(); | 
| } | 
| - return this->NewTargetExpression(scope_, factory(), pos); | 
| + return this->NewTargetExpression(scope(), factory(), pos); | 
| } | 
| template <class Traits> | 
| @@ -3354,7 +3362,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral( | 
| FunctionKind arrow_kind = is_async ? kAsyncArrowFunction : kArrowFunction; | 
| { | 
| typename Traits::Type::Factory function_factory(ast_value_factory()); | 
| - FunctionState function_state(&function_state_, &scope_, | 
| + FunctionState function_state(&function_state_, &state_, | 
| formal_parameters.scope, arrow_kind, | 
| &function_factory); | 
| @@ -3369,7 +3377,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral( | 
| // Multiple statement body | 
| Consume(Token::LBRACE); | 
| bool is_lazily_parsed = | 
| - (mode() == PARSE_LAZILY && scope_->AllowsLazyParsing()); | 
| + (mode() == PARSE_LAZILY && scope()->AllowsLazyParsing()); | 
| if (is_lazily_parsed) { | 
| body = this->NewStatementList(0, zone()); | 
| this->SkipLazyFunctionBody(&materialized_literal_count, |