From 316dc1733191538bb0df4da7dfbd7f2e6bbf2d0f Mon Sep 17 00:00:00 2001 From: adamk Date: Fri, 8 Jan 2016 12:38:07 -0800 Subject: [PATCH] Clean up FunctionLiteral AST node cruft Removed unused name_ field, made bitfield 16-bits long, and moved it to the start of the struct, resulting in a reduction of 8 bytes on both 32 and 64-bit platforms. Most other changes (which prompted this work) are cosmetic:r - Combined redundant enums - Named enum values kConsistently - Consistently use booleans in bitfield, using enum values only for passing information into NewFunctionLiteral - Removed unneeded arguments from NewFunctionLiteral, reducing clutter at callsites - Added const correctness consistently Review URL: https://codereview.chromium.org/1566053002 Cr-Commit-Position: refs/heads/master@{#33194} --- src/ast/ast.h | 97 +++++++++++++++++---------------------- src/parsing/parser-base.h | 27 ++++++----- src/parsing/parser.cc | 43 +++++++++-------- src/parsing/preparser.cc | 4 +- src/parsing/preparser.h | 7 ++- 5 files changed, 81 insertions(+), 97 deletions(-) diff --git a/src/ast/ast.h b/src/ast/ast.h index cd921eb493..ce472464b2 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -2557,30 +2557,17 @@ class Throw final : public Expression { class FunctionLiteral final : public Expression { public: enum FunctionType { - ANONYMOUS_EXPRESSION, - NAMED_EXPRESSION, - DECLARATION + kAnonymousExpression, + kNamedExpression, + kDeclaration, + kGlobalOrEval }; - enum ParameterFlag { - kNoDuplicateParameters = 0, - kHasDuplicateParameters = 1 - }; - - enum IsFunctionFlag { - kGlobalOrEval, - kIsFunction - }; + enum ParameterFlag { kNoDuplicateParameters, kHasDuplicateParameters }; enum EagerCompileHint { kShouldEagerCompile, kShouldLazyCompile }; - enum ShouldBeUsedOnceHint { kShouldBeUsedOnce, kDontKnowIfShouldBeUsedOnce }; - - enum ArityRestriction { - NORMAL_ARITY, - GETTER_ARITY, - SETTER_ARITY - }; + enum ArityRestriction { kNormalArity, kGetterArity, kSetterArity }; DECLARE_NODE_TYPE(FunctionLiteral) @@ -2641,14 +2628,14 @@ class FunctionLiteral final : public Expression { inferred_name_ = Handle(); } - bool pretenure() { return Pretenure::decode(bitfield_); } - void set_pretenure() { bitfield_ |= Pretenure::encode(true); } + bool pretenure() const { return Pretenure::decode(bitfield_); } + void set_pretenure() { bitfield_ = Pretenure::update(bitfield_, true); } - bool has_duplicate_parameters() { + bool has_duplicate_parameters() const { return HasDuplicateParameters::decode(bitfield_); } - bool is_function() { return IsFunction::decode(bitfield_) == kIsFunction; } + bool is_function() const { return IsFunction::decode(bitfield_); } // This is used as a heuristic on when to eagerly compile a function // literal. We consider the following constructs as hints that the @@ -2656,19 +2643,19 @@ class FunctionLiteral final : public Expression { // - (function() { ... })(); // - var x = function() { ... }(); bool should_eager_compile() const { - return EagerCompileHintBit::decode(bitfield_) == kShouldEagerCompile; + return ShouldEagerCompile::decode(bitfield_); } void set_should_eager_compile() { - bitfield_ = EagerCompileHintBit::update(bitfield_, kShouldEagerCompile); + bitfield_ = ShouldEagerCompile::update(bitfield_, true); } // A hint that we expect this function to be called (exactly) once, // i.e. we suspect it's an initialization function. bool should_be_used_once_hint() const { - return ShouldBeUsedOnceHintBit::decode(bitfield_) == kShouldBeUsedOnce; + return ShouldBeUsedOnceHint::decode(bitfield_); } void set_should_be_used_once_hint() { - bitfield_ = ShouldBeUsedOnceHintBit::update(bitfield_, kShouldBeUsedOnce); + bitfield_ = ShouldBeUsedOnceHint::update(bitfield_, true); } FunctionKind kind() const { return FunctionKindBits::decode(bitfield_); } @@ -2694,7 +2681,6 @@ class FunctionLiteral final : public Expression { int expected_property_count, int parameter_count, FunctionType function_type, ParameterFlag has_duplicate_parameters, - IsFunctionFlag is_function, EagerCompileHint eager_compile_hint, FunctionKind kind, int position) : Expression(zone, position), @@ -2708,20 +2694,33 @@ class FunctionLiteral final : public Expression { expected_property_count_(expected_property_count), parameter_count_(parameter_count), function_token_position_(RelocInfo::kNoPosition) { - bitfield_ = IsExpression::encode(function_type != DECLARATION) | - IsAnonymous::encode(function_type == ANONYMOUS_EXPRESSION) | - Pretenure::encode(false) | - HasDuplicateParameters::encode(has_duplicate_parameters) | - IsFunction::encode(is_function) | - EagerCompileHintBit::encode(eager_compile_hint) | - FunctionKindBits::encode(kind) | - ShouldBeUsedOnceHintBit::encode(kDontKnowIfShouldBeUsedOnce); + bitfield_ = + IsExpression::encode(function_type != kDeclaration) | + IsAnonymous::encode(function_type == kAnonymousExpression) | + Pretenure::encode(false) | + HasDuplicateParameters::encode(has_duplicate_parameters == + kHasDuplicateParameters) | + IsFunction::encode(function_type != kGlobalOrEval) | + ShouldEagerCompile::encode(eager_compile_hint == kShouldEagerCompile) | + FunctionKindBits::encode(kind) | ShouldBeUsedOnceHint::encode(false); DCHECK(IsValidFunctionKind(kind)); } private: + class IsExpression : public BitField16 {}; + class IsAnonymous : public BitField16 {}; + class Pretenure : public BitField16 {}; + class HasDuplicateParameters : public BitField16 {}; + class IsFunction : public BitField16 {}; + class ShouldEagerCompile : public BitField16 {}; + class FunctionKindBits : public BitField16 {}; + class ShouldBeUsedOnceHint : public BitField16 {}; + + // Start with 16-bit field, which should get packed together + // with Expression's trailing 16-bit field. + uint16_t bitfield_; + const AstString* raw_name_; - Handle name_; Scope* scope_; ZoneList* body_; const AstString* raw_inferred_name_; @@ -2733,17 +2732,6 @@ class FunctionLiteral final : public Expression { int expected_property_count_; int parameter_count_; int function_token_position_; - - unsigned bitfield_; - class IsExpression : public BitField {}; - class IsAnonymous : public BitField {}; - class Pretenure : public BitField {}; - class HasDuplicateParameters : public BitField {}; - class IsFunction : public BitField {}; - class EagerCompileHintBit : public BitField {}; - class FunctionKindBits : public BitField {}; - class ShouldBeUsedOnceHintBit : public BitField { - }; }; @@ -3325,19 +3313,18 @@ class AstNodeFactory final BASE_EMBEDDED { } FunctionLiteral* NewFunctionLiteral( - const AstRawString* name, AstValueFactory* ast_value_factory, - Scope* scope, ZoneList* body, int materialized_literal_count, - int expected_property_count, int parameter_count, + const AstRawString* name, Scope* scope, ZoneList* body, + int materialized_literal_count, int expected_property_count, + int parameter_count, FunctionLiteral::ParameterFlag has_duplicate_parameters, FunctionLiteral::FunctionType function_type, - FunctionLiteral::IsFunctionFlag is_function, FunctionLiteral::EagerCompileHint eager_compile_hint, FunctionKind kind, int position) { return new (parser_zone_) FunctionLiteral( - parser_zone_, name, ast_value_factory, scope, body, + parser_zone_, name, ast_value_factory_, scope, body, materialized_literal_count, expected_property_count, parameter_count, - function_type, has_duplicate_parameters, is_function, - eager_compile_hint, kind, position); + function_type, has_duplicate_parameters, eager_compile_hint, kind, + position); } ClassLiteral* NewClassLiteral(const AstRawString* name, Scope* scope, diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index 2e66c0f7fb..1aa4f3722a 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -1749,8 +1749,8 @@ ParserBase::ParsePropertyDefinition( value = this->ParseFunctionLiteral( *name, scanner()->location(), kSkipFunctionNameCheck, kind, - RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION, - FunctionLiteral::NORMAL_ARITY, language_mode(), + RelocInfo::kNoPosition, FunctionLiteral::kAnonymousExpression, + FunctionLiteral::kNormalArity, language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty)); return factory()->NewObjectLiteralProperty(name_expression, value, @@ -1789,8 +1789,8 @@ ParserBase::ParsePropertyDefinition( if (!in_class) kind = WithObjectLiteralBit(kind); typename Traits::Type::FunctionLiteral value = this->ParseFunctionLiteral( *name, scanner()->location(), kSkipFunctionNameCheck, kind, - RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION, - is_get ? FunctionLiteral::GETTER_ARITY : FunctionLiteral::SETTER_ARITY, + RelocInfo::kNoPosition, FunctionLiteral::kAnonymousExpression, + is_get ? FunctionLiteral::kGetterArity : FunctionLiteral::kSetterArity, language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty)); // Make sure the name expression is a string since we need a Name for @@ -2556,12 +2556,12 @@ ParserBase::ParseMemberExpression(ExpressionClassifier* classifier, bool is_strict_reserved_name = false; Scanner::Location function_name_location = Scanner::Location::invalid(); FunctionLiteral::FunctionType function_type = - FunctionLiteral::ANONYMOUS_EXPRESSION; + FunctionLiteral::kAnonymousExpression; if (peek_any_identifier()) { name = ParseIdentifierOrStrictReservedWord( is_generator, &is_strict_reserved_name, CHECK_OK); function_name_location = scanner()->location(); - function_type = FunctionLiteral::NAMED_EXPRESSION; + function_type = FunctionLiteral::kNamedExpression; } result = this->ParseFunctionLiteral( name, function_name_location, @@ -2569,7 +2569,7 @@ ParserBase::ParseMemberExpression(ExpressionClassifier* classifier, : kFunctionNameValidityUnknown, is_generator ? FunctionKind::kGeneratorFunction : FunctionKind::kNormalFunction, - function_token_position, function_type, FunctionLiteral::NORMAL_ARITY, + function_token_position, function_type, FunctionLiteral::kNormalArity, language_mode(), CHECK_OK); } else if (peek() == Token::SUPER) { const bool is_new = false; @@ -2942,14 +2942,14 @@ void ParserBase::CheckArityRestrictions( int param_count, FunctionLiteral::ArityRestriction arity_restriction, bool has_rest, int formals_start_pos, int formals_end_pos, bool* ok) { switch (arity_restriction) { - case FunctionLiteral::GETTER_ARITY: + case FunctionLiteral::kGetterArity: if (param_count != 0) { ReportMessageAt(Scanner::Location(formals_start_pos, formals_end_pos), MessageTemplate::kBadGetterArity); *ok = false; } break; - case FunctionLiteral::SETTER_ARITY: + case FunctionLiteral::kSetterArity: if (param_count != 1) { ReportMessageAt(Scanner::Location(formals_start_pos, formals_end_pos), MessageTemplate::kBadSetterArity); @@ -3037,7 +3037,7 @@ ParserBase::ParseArrowFunctionLiteral( } else { body = this->ParseEagerFunctionBody( this->EmptyIdentifier(), RelocInfo::kNoPosition, formal_parameters, - kArrowFunction, FunctionLiteral::ANONYMOUS_EXPRESSION, CHECK_OK); + kArrowFunction, FunctionLiteral::kAnonymousExpression, CHECK_OK); materialized_literal_count = function_state.materialized_literal_count(); expected_property_count = function_state.expected_property_count(); @@ -3081,11 +3081,10 @@ ParserBase::ParseArrowFunctionLiteral( } FunctionLiteralT function_literal = factory()->NewFunctionLiteral( - this->EmptyIdentifierString(), ast_value_factory(), - formal_parameters.scope, body, materialized_literal_count, - expected_property_count, num_parameters, + this->EmptyIdentifierString(), formal_parameters.scope, body, + materialized_literal_count, expected_property_count, num_parameters, FunctionLiteral::kNoDuplicateParameters, - FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kIsFunction, + FunctionLiteral::kAnonymousExpression, FunctionLiteral::kShouldLazyCompile, FunctionKind::kArrowFunction, formal_parameters.scope->start_position()); diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index baff20e1d5..e2d5ab20c4 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -236,10 +236,10 @@ FunctionLiteral* Parser::DefaultConstructor(bool call_super, Scope* scope, } FunctionLiteral* function_literal = factory()->NewFunctionLiteral( - name, ast_value_factory(), function_scope, body, - materialized_literal_count, expected_property_count, parameter_count, + name, function_scope, body, materialized_literal_count, + expected_property_count, parameter_count, FunctionLiteral::kNoDuplicateParameters, - FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kIsFunction, + FunctionLiteral::kAnonymousExpression, FunctionLiteral::kShouldLazyCompile, kind, pos); return function_literal; @@ -947,13 +947,12 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) { if (ok) { ParserTraits::RewriteDestructuringAssignments(); result = factory()->NewFunctionLiteral( - ast_value_factory()->empty_string(), ast_value_factory(), scope_, - body, function_state.materialized_literal_count(), + ast_value_factory()->empty_string(), scope_, body, + function_state.materialized_literal_count(), function_state.expected_property_count(), 0, FunctionLiteral::kNoDuplicateParameters, - FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kGlobalOrEval, - FunctionLiteral::kShouldLazyCompile, FunctionKind::kNormalFunction, - 0); + FunctionLiteral::kGlobalOrEval, FunctionLiteral::kShouldLazyCompile, + FunctionKind::kNormalFunction, 0); } } @@ -1039,11 +1038,12 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info, DCHECK(is_sloppy(scope->language_mode()) || is_strict(info->language_mode())); DCHECK(info->language_mode() == shared_info->language_mode()); - FunctionLiteral::FunctionType function_type = shared_info->is_expression() - ? (shared_info->is_anonymous() - ? FunctionLiteral::ANONYMOUS_EXPRESSION - : FunctionLiteral::NAMED_EXPRESSION) - : FunctionLiteral::DECLARATION; + FunctionLiteral::FunctionType function_type = + shared_info->is_expression() + ? (shared_info->is_anonymous() + ? FunctionLiteral::kAnonymousExpression + : FunctionLiteral::kNamedExpression) + : FunctionLiteral::kDeclaration; bool ok = true; if (shared_info->is_arrow()) { @@ -1105,7 +1105,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info, result = ParseFunctionLiteral( raw_name, Scanner::Location::invalid(), kSkipFunctionNameCheck, shared_info->kind(), RelocInfo::kNoPosition, function_type, - FunctionLiteral::NORMAL_ARITY, shared_info->language_mode(), &ok); + FunctionLiteral::kNormalArity, shared_info->language_mode(), &ok); } // Make sure the results agree. DCHECK(ok == (result != NULL)); @@ -2110,7 +2110,7 @@ Statement* Parser::ParseFunctionDeclaration( : kFunctionNameValidityUnknown, is_generator ? FunctionKind::kGeneratorFunction : FunctionKind::kNormalFunction, - pos, FunctionLiteral::DECLARATION, FunctionLiteral::NORMAL_ARITY, + pos, FunctionLiteral::kDeclaration, FunctionLiteral::kNormalArity, language_mode(), CHECK_OK); // Even if we're not at the top-level of the global or a function @@ -4111,7 +4111,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral( // nested function, and hoisting works normally relative to that. Scope* declaration_scope = scope_->DeclarationScope(); Scope* original_declaration_scope = original_scope_->DeclarationScope(); - Scope* scope = function_type == FunctionLiteral::DECLARATION && + Scope* scope = function_type == FunctionLiteral::kDeclaration && is_sloppy(language_mode) && !allow_harmony_sloppy_function() && (original_scope_ == original_declaration_scope || @@ -4245,7 +4245,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral( // - The function literal shouldn't be hinted to eagerly compile. bool use_temp_zone = FLAG_lazy && !allow_natives() && extension_ == NULL && allow_lazy() && - function_type == FunctionLiteral::DECLARATION && + function_type == FunctionLiteral::kDeclaration && eager_compile_hint != FunctionLiteral::kShouldEagerCompile; // Open a new BodyScope, which sets our AstNodeFactory to allocate in the // new temporary zone if the preconditions are satisfied, and ensures that @@ -4317,9 +4317,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral( : FunctionLiteral::kNoDuplicateParameters; FunctionLiteral* function_literal = factory()->NewFunctionLiteral( - function_name, ast_value_factory(), scope, body, - materialized_literal_count, expected_property_count, arity, - duplicate_parameters, function_type, FunctionLiteral::kIsFunction, + function_name, scope, body, materialized_literal_count, + expected_property_count, arity, duplicate_parameters, function_type, eager_compile_hint, kind, pos); function_literal->set_function_token_position(function_token_pos); if (should_be_used_once_hint) @@ -4559,7 +4558,7 @@ ZoneList* Parser::ParseEagerFunctionBody( ZoneList* result = new(zone()) ZoneList(8, zone()); static const int kFunctionNameAssignmentIndex = 0; - if (function_type == FunctionLiteral::NAMED_EXPRESSION) { + if (function_type == FunctionLiteral::kNamedExpression) { DCHECK(function_name != NULL); // If we have a named function expression, we add a local variable // declaration to the body of the function with the name of the @@ -4646,7 +4645,7 @@ ZoneList* Parser::ParseEagerFunctionBody( result->Add(inner_block, zone()); } - if (function_type == FunctionLiteral::NAMED_EXPRESSION) { + if (function_type == FunctionLiteral::kNamedExpression) { // Now that we know the language mode, we can create the const assignment // in the previously reserved spot. // NOTE: We create a proxy and resolve it here so that in the diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc index cca31a9bc7..64511acc39 100644 --- a/src/parsing/preparser.cc +++ b/src/parsing/preparser.cc @@ -451,8 +451,8 @@ PreParser::Statement PreParser::ParseFunctionDeclaration(bool* ok) { : kFunctionNameValidityUnknown, is_generator ? FunctionKind::kGeneratorFunction : FunctionKind::kNormalFunction, - pos, FunctionLiteral::DECLARATION, - FunctionLiteral::NORMAL_ARITY, language_mode(), + pos, FunctionLiteral::kDeclaration, + FunctionLiteral::kNormalArity, language_mode(), CHECK_OK); return Statement::FunctionDeclaration(); } diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h index 41d5ab8513..d9f016d221 100644 --- a/src/parsing/preparser.h +++ b/src/parsing/preparser.h @@ -540,12 +540,11 @@ class PreParserFactory { return PreParserStatement::Default(); } PreParserExpression NewFunctionLiteral( - PreParserIdentifier name, AstValueFactory* ast_value_factory, - Scope* scope, PreParserStatementList body, int materialized_literal_count, - int expected_property_count, int parameter_count, + PreParserIdentifier name, Scope* scope, PreParserStatementList body, + int materialized_literal_count, int expected_property_count, + int parameter_count, FunctionLiteral::ParameterFlag has_duplicate_parameters, FunctionLiteral::FunctionType function_type, - FunctionLiteral::IsFunctionFlag is_function, FunctionLiteral::EagerCompileHint eager_compile_hint, FunctionKind kind, int position) { return PreParserExpression::Default();