From b86c30a2b3e361a59decfbbe1dc0b90b9b1945bd Mon Sep 17 00:00:00 2001 From: "arv@chromium.org" Date: Fri, 7 Nov 2014 16:39:00 +0000 Subject: [PATCH] Classes: Partial fix for constructor not calling super Introduce two new function kind, one for default constructor and one for default constructor call super. Then when we are about to pares these we just generate the correct AST in source. BUG=v8:3661, v8:3672 LOG=Y R=dslomov@chromium.org Review URL: https://codereview.chromium.org/700523003 Cr-Commit-Position: refs/heads/master@{#25222} git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25222 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast.h | 8 ++++- src/code-stubs.h | 6 +++- src/full-codegen.cc | 6 +--- src/globals.h | 20 ++++++++++-- src/hydrogen-instructions.h | 12 +++++--- src/objects-inl.h | 5 +++ src/objects.h | 11 ++++++- src/parser.cc | 68 +++++++++++++++++++++++++++++++++++++++++ src/parser.h | 6 +++- src/preparser.h | 13 ++++++++ src/runtime/runtime-classes.cc | 53 ++++++++++++++------------------ test/mjsunit/harmony/classes.js | 37 +++++++++++++++++++++- 12 files changed, 199 insertions(+), 46 deletions(-) diff --git a/src/ast.h b/src/ast.h index 749e579..1700916 100644 --- a/src/ast.h +++ b/src/ast.h @@ -2576,6 +2576,12 @@ class FunctionLiteral FINAL : public Expression { bool is_concise_method() { return IsConciseMethod(FunctionKindBits::decode(bitfield_)); } + bool is_default_constructor() { + return IsDefaultConstructor(FunctionKindBits::decode(bitfield_)); + } + bool is_default_constructor_call_super() { + return IsDefaultConstructorCallSuper(FunctionKindBits::decode(bitfield_)); + } int ast_node_count() { return ast_properties_.node_count(); } AstProperties::Flags* flags() { return ast_properties_.flags(); } @@ -2647,7 +2653,7 @@ class FunctionLiteral FINAL : public Expression { class HasDuplicateParameters : public BitField {}; class IsFunction : public BitField {}; class IsParenthesized : public BitField {}; - class FunctionKindBits : public BitField {}; + class FunctionKindBits : public BitField {}; }; diff --git a/src/code-stubs.h b/src/code-stubs.h index 06eff69..13f8e42 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -577,10 +577,14 @@ class FastNewClosureStub : public HydrogenCodeStub { bool is_arrow() const { return IsArrowFunction(kind()); } bool is_generator() const { return IsGeneratorFunction(kind()); } bool is_concise_method() const { return IsConciseMethod(kind()); } + bool is_default_constructor() const { return IsDefaultConstructor(kind()); } + bool is_default_constructor_call_super() const { + return IsDefaultConstructorCallSuper(kind()); + } private: class StrictModeBits : public BitField {}; - class FunctionKindBits : public BitField {}; + class FunctionKindBits : public BitField {}; DEFINE_CALL_INTERFACE_DESCRIPTOR(FastNewClosure); DEFINE_HYDROGEN_CODE_STUB(FastNewClosure, HydrogenCodeStub); diff --git a/src/full-codegen.cc b/src/full-codegen.cc index 58e5e97..01f2faf 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -1575,11 +1575,7 @@ void FullCodeGenerator::VisitClassLiteral(ClassLiteral* lit) { __ Push(isolate()->factory()->the_hole_value()); } - if (lit->constructor() != NULL) { - VisitForStackValue(lit->constructor()); - } else { - __ Push(isolate()->factory()->undefined_value()); - } + VisitForStackValue(lit->constructor()); __ Push(script()); __ Push(Smi::FromInt(lit->start_position())); diff --git a/src/globals.h b/src/globals.h index c6ba010..7fa4317 100644 --- a/src/globals.h +++ b/src/globals.h @@ -776,7 +776,9 @@ enum FunctionKind { kArrowFunction = 1, kGeneratorFunction = 2, kConciseMethod = 4, - kConciseGeneratorMethod = kGeneratorFunction | kConciseMethod + kConciseGeneratorMethod = kGeneratorFunction | kConciseMethod, + kDefaultConstructor = 8, + kDefaultConstructorCallSuper = 16 }; @@ -785,7 +787,9 @@ inline bool IsValidFunctionKind(FunctionKind kind) { kind == FunctionKind::kArrowFunction || kind == FunctionKind::kGeneratorFunction || kind == FunctionKind::kConciseMethod || - kind == FunctionKind::kConciseGeneratorMethod; + kind == FunctionKind::kConciseGeneratorMethod || + kind == FunctionKind::kDefaultConstructor || + kind == FunctionKind::kDefaultConstructorCallSuper; } @@ -805,6 +809,18 @@ inline bool IsConciseMethod(FunctionKind kind) { DCHECK(IsValidFunctionKind(kind)); return kind & FunctionKind::kConciseMethod; } + + +inline bool IsDefaultConstructor(FunctionKind kind) { + DCHECK(IsValidFunctionKind(kind)); + return kind & FunctionKind::kDefaultConstructor; +} + + +inline bool IsDefaultConstructorCallSuper(FunctionKind kind) { + DCHECK(IsValidFunctionKind(kind)); + return kind & FunctionKind::kDefaultConstructorCallSuper; +} } } // namespace v8::internal namespace i = v8::internal; diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 233ca42..9101576 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -7562,6 +7562,10 @@ class HFunctionLiteral FINAL : public HTemplateInstruction<1> { bool is_arrow() const { return IsArrowFunction(kind()); } bool is_generator() const { return IsGeneratorFunction(kind()); } bool is_concise_method() const { return IsConciseMethod(kind()); } + bool is_default_constructor() const { return IsDefaultConstructor(kind()); } + bool is_default_constructor_call_super() const { + return IsDefaultConstructorCallSuper(kind()); + } FunctionKind kind() const { return FunctionKindField::decode(bit_field_); } StrictMode strict_mode() const { return StrictModeField::decode(bit_field_); } @@ -7581,10 +7585,10 @@ class HFunctionLiteral FINAL : public HTemplateInstruction<1> { virtual bool IsDeletable() const OVERRIDE { return true; } - class FunctionKindField : public BitField {}; - class PretenureField : public BitField {}; - class HasNoLiteralsField : public BitField {}; - class StrictModeField : public BitField {}; + class FunctionKindField : public BitField {}; + class PretenureField : public BitField {}; + class HasNoLiteralsField : public BitField {}; + class StrictModeField : public BitField {}; Handle shared_info_; uint32_t bit_field_; diff --git a/src/objects-inl.h b/src/objects-inl.h index 7ba4778..40ce81a 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5695,6 +5695,11 @@ BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_arrow, kIsArrow) BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_generator, kIsGenerator) BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_concise_method, kIsConciseMethod) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_default_constructor, + kIsDefaultConstructor) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, + is_default_constructor_call_super, + kIsDefaultConstructorCallSuper) ACCESSORS(CodeCache, default_cache, FixedArray, kDefaultCacheOffset) ACCESSORS(CodeCache, normal_type_cache, Object, kNormalTypeCacheOffset) diff --git a/src/objects.h b/src/objects.h index 5355c4b..31d0a16 100644 --- a/src/objects.h +++ b/src/objects.h @@ -6872,6 +6872,13 @@ class SharedFunctionInfo: public HeapObject { // Indicates that this function is a concise method. DECL_BOOLEAN_ACCESSORS(is_concise_method) + // Indicates that this function is a default constructor. + DECL_BOOLEAN_ACCESSORS(is_default_constructor) + + // Indicates that this function is a default constructor that needs to call + // super. + DECL_BOOLEAN_ACCESSORS(is_default_constructor_call_super) + // Indicates that this function is an asm function. DECL_BOOLEAN_ACCESSORS(asm_function) @@ -7114,12 +7121,14 @@ class SharedFunctionInfo: public HeapObject { kIsArrow, kIsGenerator, kIsConciseMethod, + kIsDefaultConstructor, + kIsDefaultConstructorCallSuper, kIsAsmFunction, kDeserialized, kCompilerHintsCount // Pseudo entry }; - class FunctionKindBits : public BitField {}; + class FunctionKindBits : public BitField {}; class DeoptCountBits : public BitField {}; class OptReenableTriesBits : public BitField {}; diff --git a/src/parser.cc b/src/parser.cc index c5bf0d9..e34854f 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -275,6 +275,62 @@ Scope* Parser::NewScope(Scope* parent, ScopeType scope_type) { } +FunctionLiteral* Parser::DefaultConstructor(bool call_super, Scope* scope, + int pos, int end_pos) { + int materialized_literal_count = -1; + int expected_property_count = -1; + int handler_count = 0; + int parameter_count = 0; + AstProperties ast_properties; + BailoutReason dont_optimize_reason = kNoReason; + const AstRawString* name = ast_value_factory()->empty_string(); + FunctionKind kind = call_super ? FunctionKind::kDefaultConstructorCallSuper + : FunctionKind::kDefaultConstructor; + + Scope* function_scope = NewScope(scope, FUNCTION_SCOPE); + function_scope->SetStrictMode(STRICT); + // Set start and end position to the same value + function_scope->set_start_position(pos); + function_scope->set_end_position(pos); + ZoneList* body = NULL; + + { + AstNodeFactory function_factory( + ast_value_factory()); + FunctionState function_state(&function_state_, &scope_, function_scope, + &function_factory); + + body = new (zone()) ZoneList(1, zone()); + if (call_super) { + Expression* prop = SuperReference(function_scope, factory(), pos); + ZoneList* args = + new (zone()) ZoneList(0, zone()); + Call* call = factory()->NewCall(prop, args, pos); + body->Add(factory()->NewExpressionStatement(call, pos), zone()); + } + + materialized_literal_count = function_state.materialized_literal_count(); + expected_property_count = function_state.expected_property_count(); + handler_count = function_state.handler_count(); + + ast_properties = *factory()->visitor()->ast_properties(); + dont_optimize_reason = factory()->visitor()->dont_optimize_reason(); + } + + FunctionLiteral* function_literal = factory()->NewFunctionLiteral( + name, ast_value_factory(), function_scope, body, + materialized_literal_count, expected_property_count, handler_count, + parameter_count, FunctionLiteral::kNoDuplicateParameters, + FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kIsFunction, + FunctionLiteral::kNotParenthesized, kind, pos); + + function_literal->set_ast_properties(&ast_properties); + function_literal->set_dont_optimize_reason(dont_optimize_reason); + + return function_literal; +} + + // ---------------------------------------------------------------------------- // Target is a support class to facilitate manipulation of the // Parser's target_stack_ (the stack of potential 'break' and @@ -648,6 +704,13 @@ Expression* ParserTraits::ClassExpression( start_position, end_position); } + +Expression* ParserTraits::DefaultConstructor(bool call_super, Scope* scope, + int pos, int end_pos) { + return parser_->DefaultConstructor(call_super, scope, pos, end_pos); +} + + Literal* ParserTraits::ExpressionFromLiteral( Token::Value token, int pos, Scanner* scanner, @@ -1004,6 +1067,11 @@ FunctionLiteral* Parser::ParseLazy(Utf16CharacterStream* source) { Expression* expression = ParseExpression(false, &ok); DCHECK(expression->IsFunctionLiteral()); result = expression->AsFunctionLiteral(); + } else if (shared_info->is_default_constructor() || + shared_info->is_default_constructor_call_super()) { + result = DefaultConstructor( + shared_info->is_default_constructor_call_super(), scope, + shared_info->start_position(), shared_info->end_position()); } else { result = ParseFunctionLiteral(raw_name, Scanner::Location::invalid(), false, // Strict mode name already checked. diff --git a/src/parser.h b/src/parser.h index db9071a..c815a45 100644 --- a/src/parser.h +++ b/src/parser.h @@ -547,7 +547,8 @@ class ParserTraits { ZoneList* properties, int start_position, int end_position, AstNodeFactory* factory); - + Expression* DefaultConstructor(bool call_super, Scope* scope, int pos, + int end_pos); Literal* ExpressionFromLiteral( Token::Value token, int pos, Scanner* scanner, AstNodeFactory* factory); @@ -804,6 +805,9 @@ class Parser : public ParserBase { Scope* NewScope(Scope* parent, ScopeType type); + FunctionLiteral* DefaultConstructor(bool call_super, Scope* scope, int pos, + int end_pos); + // Skip over a lazy function, either using cached data if we have it, or // by parsing the function with PreParser. Consumes the ending }. void SkipLazyFunctionBody(const AstRawString* function_name, diff --git a/src/preparser.h b/src/preparser.h index ddf9cec..7c9791f 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -1325,6 +1325,12 @@ class PreParserTraits { return PreParserExpression::Default(); } + static PreParserExpression DefaultConstructor(bool call_super, + PreParserScope* scope, int pos, + int end_pos) { + return PreParserExpression::Default(); + } + static PreParserExpression ExpressionFromLiteral( Token::Value token, int pos, Scanner* scanner, PreParserFactory* factory) { @@ -2747,12 +2753,14 @@ typename ParserBase::ExpressionT ParserBase::ParseClassLiteral( return this->EmptyExpression(); } + bool has_extends = false; ExpressionT extends = this->EmptyExpression(); if (Check(Token::EXTENDS)) { typename Traits::Type::ScopePtr scope = this->NewScope(scope_, BLOCK_SCOPE); BlockState block_state(&scope_, Traits::Type::ptr_to_scope(scope)); scope_->SetStrictMode(STRICT); extends = this->ParseLeftHandSideExpression(CHECK_OK); + has_extends = true; } // TODO(arv): Implement scopes and name binding in class body only. @@ -2791,6 +2799,11 @@ typename ParserBase::ExpressionT ParserBase::ParseClassLiteral( int end_pos = peek_position(); Expect(Token::RBRACE, CHECK_OK); + if (!has_seen_constructor) { + constructor = + this->DefaultConstructor(has_extends, scope_, pos, end_pos + 1); + } + return this->ClassExpression(name, extends, constructor, properties, pos, end_pos + 1, factory()); } diff --git a/src/runtime/runtime-classes.cc b/src/runtime/runtime-classes.cc index cc4e09b..30ff918 100644 --- a/src/runtime/runtime-classes.cc +++ b/src/runtime/runtime-classes.cc @@ -62,7 +62,7 @@ RUNTIME_FUNCTION(Runtime_DefineClass) { DCHECK(args.length() == 6); CONVERT_ARG_HANDLE_CHECKED(Object, name, 0); CONVERT_ARG_HANDLE_CHECKED(Object, super_class, 1); - CONVERT_ARG_HANDLE_CHECKED(Object, constructor, 2); + CONVERT_ARG_HANDLE_CHECKED(JSFunction, constructor, 2); CONVERT_ARG_HANDLE_CHECKED(Script, script, 3); CONVERT_SMI_ARG_CHECKED(start_position, 4); CONVERT_SMI_ARG_CHECKED(end_position, 5); @@ -104,52 +104,45 @@ RUNTIME_FUNCTION(Runtime_DefineClass) { Handle name_string = name->IsString() ? Handle::cast(name) : isolate->factory()->empty_string(); + constructor->shared()->set_name(*name_string); - Handle ctor; - if (constructor->IsSpecFunction()) { - ctor = Handle::cast(constructor); - JSFunction::SetPrototype(ctor, prototype); - PropertyAttributes attribs = - static_cast(DONT_ENUM | DONT_DELETE | READ_ONLY); - RETURN_FAILURE_ON_EXCEPTION( - isolate, - JSObject::SetOwnPropertyIgnoreAttributes( - ctor, isolate->factory()->prototype_string(), prototype, attribs)); - } else { - // TODO(arv): This should not use an empty function but a function that - // calls super. - Handle code(isolate->builtins()->builtin(Builtins::kEmptyFunction)); - ctor = isolate->factory()->NewFunction(name_string, code, prototype, true); - } - + JSFunction::SetPrototype(constructor, prototype); + PropertyAttributes attribs = + static_cast(DONT_ENUM | DONT_DELETE | READ_ONLY); + RETURN_FAILURE_ON_EXCEPTION( + isolate, JSObject::SetOwnPropertyIgnoreAttributes( + constructor, isolate->factory()->prototype_string(), + prototype, attribs)); Handle home_object_symbol(isolate->heap()->home_object_symbol()); RETURN_FAILURE_ON_EXCEPTION( isolate, JSObject::SetOwnPropertyIgnoreAttributes( - ctor, home_object_symbol, prototype, DONT_ENUM)); + constructor, home_object_symbol, prototype, DONT_ENUM)); if (!constructor_parent.is_null()) { RETURN_FAILURE_ON_EXCEPTION( - isolate, JSObject::SetPrototype(ctor, constructor_parent, false)); + isolate, + JSObject::SetPrototype(constructor, constructor_parent, false)); } JSObject::AddProperty(prototype, isolate->factory()->constructor_string(), - ctor, DONT_ENUM); + constructor, DONT_ENUM); // Install private properties that are used to construct the FunctionToString. RETURN_FAILURE_ON_EXCEPTION( + isolate, Object::SetProperty(constructor, + isolate->factory()->class_script_symbol(), + script, STRICT)); + RETURN_FAILURE_ON_EXCEPTION( isolate, - Object::SetProperty(ctor, isolate->factory()->class_script_symbol(), - script, STRICT)); + Object::SetProperty( + constructor, isolate->factory()->class_start_position_symbol(), + handle(Smi::FromInt(start_position), isolate), STRICT)); RETURN_FAILURE_ON_EXCEPTION( isolate, Object::SetProperty( - ctor, isolate->factory()->class_start_position_symbol(), - handle(Smi::FromInt(start_position), isolate), STRICT)); - RETURN_FAILURE_ON_EXCEPTION( - isolate, - Object::SetProperty(ctor, isolate->factory()->class_end_position_symbol(), - handle(Smi::FromInt(end_position), isolate), STRICT)); + constructor, isolate->factory()->class_end_position_symbol(), + handle(Smi::FromInt(end_position), isolate), STRICT)); - return *ctor; + return *constructor; } diff --git a/test/mjsunit/harmony/classes.js b/test/mjsunit/harmony/classes.js index 59371e4..8748f62 100644 --- a/test/mjsunit/harmony/classes.js +++ b/test/mjsunit/harmony/classes.js @@ -19,8 +19,16 @@ assertEquals(Function.prototype, Object.getPrototypeOf(D)); assertEquals('D', D.name); + class D2 { constructor() {} } + assertEquals('D2', D2.name); + + // TODO(arv): The logic for the name of anonymous functions in ES6 requires + // the below to be 'E'; var E = class {} - assertEquals('', E.name); + assertEquals('', E.name); // Should be 'E'. + + var F = class { constructor() {} }; + assertEquals('', F.name); // Should be 'F'. })(); @@ -589,6 +597,33 @@ function assertAccessorDescriptor(object, name) { })(); +(function TestDefaultConstructorNoCrash() { + // Regression test for https://code.google.com/p/v8/issues/detail?id=3661 + class C {} + assertEquals(undefined, C()); + assertEquals(undefined, C(1)); + assertTrue(new C() instanceof C); + assertTrue(new C(1) instanceof C); +})(); + + +(function TestDefaultConstructor() { + var calls = 0; + class Base { + constructor() { + calls++; + } + } + class Derived extends Base {} + var object = new Derived; + assertEquals(1, calls); + + calls = 0; + Derived(); + assertEquals(1, calls); +})(); + + /* TODO(arv): Implement (function TestNameBindingInConstructor() { class C { -- 2.7.4