From d888297d71e96072a529721eb64a1bc77f814d1b Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Thu, 20 Apr 2017 15:22:17 -0700 Subject: [PATCH] Bug 1357506 - Remove assert that constructorBox can only be set once when parsing classes. (r=Yoric) Both asm.js and syntax parsing can abort and rewind parsing of an inner function. The bookkeeping to make sure that a class's constructor FunctionBox is only set once is not worth it -- duplicate constructor definitions already throw an early error. --- js/src/frontend/Parser.cpp | 17 +++-------------- js/src/frontend/Parser.h | 21 +++------------------ js/src/frontend/SharedContext.h | 1 - js/src/jit-test/tests/class/bug1357506.js | 8 ++++++++ 4 files changed, 14 insertions(+), 33 deletions(-) create mode 100644 js/src/jit-test/tests/class/bug1357506.js diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index e14270a454c2..13e953342f1c 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -548,7 +548,7 @@ FunctionBox::initWithEnclosingParseContext(ParseContext* enclosing, FunctionSynt if (kind == ClassConstructor || kind == DerivedClassConstructor) { auto stmt = enclosing->findInnermostStatement(); MOZ_ASSERT(stmt); - stmt->setConstructorBox(this); + stmt->constructorBox = this; if (kind == DerivedClassConstructor) { setDerivedClassConstructor(); @@ -574,16 +574,6 @@ FunctionBox::initWithEnclosingParseContext(ParseContext* enclosing, FunctionSynt } } -void -FunctionBox::resetForAbortedSyntaxParse(ParseContext* enclosing, FunctionSyntaxKind kind) -{ - if (kind == ClassConstructor || kind == DerivedClassConstructor) { - auto stmt = enclosing->findInnermostStatement(); - MOZ_ASSERT(stmt); - stmt->clearConstructorBoxForAbortedSyntaxParse(this); - } -} - void FunctionBox::initWithEnclosingScope(Scope* enclosingScope) { @@ -3455,7 +3445,6 @@ Parser::trySyntaxParseInnerFunction(ParseNode* pn, HandleFunct // correctness. parser->clearAbortedSyntaxParse(); usedNames.rewind(token); - funbox->resetForAbortedSyntaxParse(pc, kind); MOZ_ASSERT_IF(!parser->context->helperThread(), !parser->context->isExceptionPending()); break; @@ -7168,7 +7157,7 @@ Parser::classDefinition(YieldHandling yieldHandling, errorAt(nameOffset, JSMSG_BAD_METHOD_DEF); return null(); } - if (classStmt.constructorBox()) { + if (classStmt.constructorBox) { errorAt(nameOffset, JSMSG_DUPLICATE_PROPERTY, "constructor"); return null(); } @@ -7215,7 +7204,7 @@ Parser::classDefinition(YieldHandling yieldHandling, // Amend the toStringEnd offset for the constructor now that we've // finished parsing the class. uint32_t classEndOffset = pos().end; - if (FunctionBox* ctorbox = classStmt.constructorBox()) { + if (FunctionBox* ctorbox = classStmt.constructorBox) { if (ctorbox->function()->isInterpretedLazy()) ctorbox->function()->lazyScript()->setToStringEnd(classEndOffset); ctorbox->toStringEnd = classEndOffset; diff --git a/js/src/frontend/Parser.h b/js/src/frontend/Parser.h index b4a54ded8328..0792a22fd219 100644 --- a/js/src/frontend/Parser.h +++ b/js/src/frontend/Parser.h @@ -87,29 +87,14 @@ class ParseContext : public Nestable } }; - class ClassStatement : public Statement + struct ClassStatement : public Statement { - FunctionBox* constructorBox_; + FunctionBox* constructorBox; - public: explicit ClassStatement(ParseContext* pc) : Statement(pc, StatementKind::Class), - constructorBox_(nullptr) + constructorBox(nullptr) { } - - void clearConstructorBoxForAbortedSyntaxParse(FunctionBox* funbox) { - MOZ_ASSERT(constructorBox_ == funbox); - constructorBox_ = nullptr; - } - - void setConstructorBox(FunctionBox* funbox) { - MOZ_ASSERT(!constructorBox_); - constructorBox_ = funbox; - } - - FunctionBox* constructorBox() const { - return constructorBox_; - } }; // The intra-function scope stack. diff --git a/js/src/frontend/SharedContext.h b/js/src/frontend/SharedContext.h index c93e1899a724..1f06c286f518 100644 --- a/js/src/frontend/SharedContext.h +++ b/js/src/frontend/SharedContext.h @@ -503,7 +503,6 @@ class FunctionBox : public ObjectBox, public SharedContext void initFromLazyFunction(); void initStandaloneFunction(Scope* enclosingScope); void initWithEnclosingParseContext(ParseContext* enclosing, FunctionSyntaxKind kind); - void resetForAbortedSyntaxParse(ParseContext* enclosing, FunctionSyntaxKind kind); ObjectBox* toObjectBox() override { return this; } JSFunction* function() const { return &object->as(); } diff --git a/js/src/jit-test/tests/class/bug1357506.js b/js/src/jit-test/tests/class/bug1357506.js new file mode 100644 index 000000000000..52a5643e6255 --- /dev/null +++ b/js/src/jit-test/tests/class/bug1357506.js @@ -0,0 +1,8 @@ +// Test that constructors that abort due to asm.js do not assert due to the +// parser keeping track of the FunctionBox corresponding to the constructor. + +class a { + constructor() { + "use asm"; + } +}