Skip to content

Commit

Permalink
Bug 1635432 - Create FunctionBox without a FunctionCreationData. r=mg…
Browse files Browse the repository at this point in the history 
…audet

Initialize the FunctionBox fields from parameters instead of a
FunctionCreationData instance, and allow creating the JSFunction directly
from the FunctionBox. This allows us to remove most fields from the
FunctionCreationData type.

Note that we still use a default instance of FunctionCreationData to
correctly fill in CompilationInfo::funcData list. Future work will address
that when we unify with ScriptStencil.

Differential Revision: https://phabricator.services.mozilla.com/D73883
  • Loading branch information
@moztcampbell
moztcampbell committed May 6, 2020
1 parent 06d9dd2 commit 067091a
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 184 deletions.
19 changes: 14 additions & 5 deletions js/src/frontend/BinASTParserPerTokenizer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,20 @@ JS::Result<FunctionBox*> BinASTParserPerTokenizer<Tok>::buildFunctionBox(
// Allocate the function before walking down the tree.
RootedFunction fun(cx_);
if (pc_) {
Rooted<FunctionCreationData> fcd(
cx_,
FunctionCreationData(atom, syntax, generatorKind, functionAsyncKind));
BINJS_TRY_VAR(fun, AllocNewFunction(cx_, 0, fcd));
MOZ_ASSERT(fun->explicitName() == atom);
FunctionFlags flags =
InitialFunctionFlags(syntax, generatorKind, functionAsyncKind);

RootedObject proto(cx_);
BINJS_TRY(
GetFunctionPrototype(cx_, generatorKind, functionAsyncKind, &proto));

gc::AllocKind allocKind = flags.isExtended()
? gc::AllocKind::FUNCTION_EXTENDED
: gc::AllocKind::FUNCTION;

BINJS_TRY_VAR(
fun, NewFunctionWithProto(cx_, nullptr, 0, flags, nullptr, atom, proto,
allocKind, TenuredObject));
} else {
BINJS_TRY_VAR(fun, lazyScript_->function());
}
Expand Down
173 changes: 65 additions & 108 deletions js/src/frontend/Parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,14 @@ FunctionBox* PerHandlerParser<ParseHandler>::newFunctionBox(

template <class ParseHandler>
FunctionBox* PerHandlerParser<ParseHandler>::newFunctionBox(
FunctionNodeType funNode, Handle<FunctionCreationData> fcd,
FunctionNodeType funNode, HandleAtom explicitName, FunctionFlags flags,
uint32_t toStringStart, Directives inheritedDirectives,
GeneratorKind generatorKind, FunctionAsyncKind asyncKind) {
MOZ_ASSERT(funNode);

size_t index = this->getCompilationInfo().funcData.length();
if (!this->getCompilationInfo().funcData.emplaceBack(
mozilla::AsVariant(fcd.get()))) {
mozilla::AsVariant(FunctionCreationData()))) {
return nullptr;
}

Expand All @@ -320,8 +320,8 @@ FunctionBox* PerHandlerParser<ParseHandler>::newFunctionBox(
*/
FunctionBox* funbox = alloc_.new_<FunctionBox>(
cx_, traceListHead_, extent, this->getCompilationInfo(),
inheritedDirectives, generatorKind, asyncKind, fcd.get().explicitName,
fcd.get().flags, index);
inheritedDirectives, generatorKind, asyncKind, explicitName, flags,
index);

if (!funbox) {
ReportOutOfMemory(cx_);
Expand Down Expand Up @@ -1804,8 +1804,7 @@ bool ParserBase::publishDeferredFunctions(FunctionTree* root) {
Rooted<FunctionCreationData> fcd(
parser->cx_, std::move(funbox->functionCreationData().get()));

RootedFunction fun(parser->cx_,
AllocNewFunction(parser->cx_, funbox->nargs(), fcd));
RootedFunction fun(parser->cx_, funbox->createFunction(parser->cx_));
if (!fun) {
return false;
}
Expand Down Expand Up @@ -2071,20 +2070,16 @@ GeneralParser<ParseHandler, Unit>::functionBody(InHandling inHandling,
return finishLexicalScope(pc_->varScope(), body, ScopeKind::FunctionLexical);
}

FunctionCreationData::FunctionCreationData(HandleAtom explicitName,
FunctionSyntaxKind kind,
GeneratorKind generatorKind,
FunctionAsyncKind asyncKind,
bool isSelfHosting /* = false */,
bool inFunctionBox /* = false */)
: explicitName(explicitName) {
bool isExtendedUnclonedSelfHostedFunctionName =
isSelfHosting && explicitName &&
IsExtendedUnclonedSelfHostedFunctionName(explicitName);
MOZ_ASSERT_IF(isExtendedUnclonedSelfHostedFunctionName, !inFunctionBox);

FunctionFlags InitialFunctionFlags(FunctionSyntaxKind kind,
GeneratorKind generatorKind,
FunctionAsyncKind asyncKind,
bool isSelfHosting, bool hasUnclonedName) {
FunctionFlags flags = {};
gc::AllocKind allocKind = gc::AllocKind::FUNCTION;

// The SetCanonicalName mechanism is only allowed on normal functions.
MOZ_ASSERT_IF(hasUnclonedName, kind == FunctionSyntaxKind::Statement);

switch (kind) {
case FunctionSyntaxKind::Expression:
flags = (generatorKind == GeneratorKind::NotGenerator &&
Expand Down Expand Up @@ -2115,7 +2110,7 @@ FunctionCreationData::FunctionCreationData(HandleAtom explicitName,
break;
default:
MOZ_ASSERT(kind == FunctionSyntaxKind::Statement);
if (isExtendedUnclonedSelfHostedFunctionName) {
if (hasUnclonedName) {
allocKind = gc::AllocKind::FUNCTION_EXTENDED;
}
flags = (generatorKind == GeneratorKind::NotGenerator &&
Expand All @@ -2132,50 +2127,7 @@ FunctionCreationData::FunctionCreationData(HandleAtom explicitName,
flags.setIsExtended();
}

if (generatorKind == GeneratorKind::Generator) {
immutableFlags.setFlag(ImmutableScriptFlagsEnum::IsGenerator);
}

if (asyncKind == FunctionAsyncKind::AsyncFunction) {
immutableFlags.setFlag(ImmutableScriptFlagsEnum::IsAsync);
}
}

HandleAtom FunctionCreationData::getExplicitName(JSContext* cx) const {
// We can create a handle here because atoms are traced
// by FunctionCreationData.
return HandleAtom::fromMarkedLocation(&explicitName);
}

JSFunction* AllocNewFunction(JSContext* cx, uint16_t nargs,
Handle<FunctionCreationData> dataHandle) {
// FunctionCreationData don't move, so it is safe to grab a reference
// out of the handle.
const FunctionCreationData& data = dataHandle.get();

RootedObject proto(cx);
GeneratorKind generatorKind =
data.immutableFlags.hasFlag(ImmutableScriptFlagsEnum::IsGenerator)
? GeneratorKind::Generator
: GeneratorKind::NotGenerator;
FunctionAsyncKind asyncKind =
data.immutableFlags.hasFlag(ImmutableScriptFlagsEnum::IsAsync)
? FunctionAsyncKind::AsyncFunction
: FunctionAsyncKind::SyncFunction;
if (!GetFunctionPrototype(cx, generatorKind, asyncKind, &proto)) {
return nullptr;
}
gc::AllocKind allocKind = data.flags.isExtended()
? gc::AllocKind::FUNCTION_EXTENDED
: gc::AllocKind::FUNCTION;
RootedFunction fun(cx, NewFunctionWithProto(cx, nullptr, nargs, data.flags,
nullptr, data.getExplicitName(cx),
proto, allocKind, TenuredObject));
if (!fun) {
return nullptr;
}

return fun;
return flags;
}

template <class ParseHandler, typename Unit>
Expand Down Expand Up @@ -2783,10 +2735,13 @@ GeneralParser<ParseHandler, Unit>::functionDefinition(
return funNode;
}

Rooted<FunctionCreationData> fcd(
cx_,
FunctionCreationData(funName, kind, generatorKind, asyncKind,
options().selfHostingMode, pc_->isFunctionBox()));
bool isSelfHosting = options().selfHostingMode;
bool hasUnclonedName = isSelfHosting && funName &&
IsExtendedUnclonedSelfHostedFunctionName(funName);
MOZ_ASSERT_IF(hasUnclonedName, !pc_->isFunctionBox());

FunctionFlags flags = InitialFunctionFlags(kind, generatorKind, asyncKind,
isSelfHosting, hasUnclonedName);

// Speculatively parse using the directives of the parent parsing context.
// If a directive is encountered (e.g., "use strict") that changes how the
Expand All @@ -2801,9 +2756,10 @@ GeneralParser<ParseHandler, Unit>::functionDefinition(
// reparse a function due to failed syntax parsing and encountering new
// "use foo" directives.
while (true) {
if (trySyntaxParseInnerFunction(
&funNode, fcd, toStringStart, inHandling, yieldHandling, kind,
generatorKind, asyncKind, tryAnnexB, directives, &newDirectives)) {
if (trySyntaxParseInnerFunction(&funNode, funName, flags, toStringStart,
inHandling, yieldHandling, kind,
generatorKind, asyncKind, tryAnnexB,
directives, &newDirectives)) {
break;
}

Expand Down Expand Up @@ -2848,7 +2804,7 @@ bool Parser<FullParseHandler, Unit>::advancePastSyntaxParsedFunction(

template <typename Unit>
bool Parser<FullParseHandler, Unit>::trySyntaxParseInnerFunction(
FunctionNode** funNode, Handle<FunctionCreationData> fcd,
FunctionNode** funNode, HandleAtom explicitName, FunctionFlags flags,
uint32_t toStringStart, InHandling inHandling, YieldHandling yieldHandling,
FunctionSyntaxKind kind, GeneratorKind generatorKind,
FunctionAsyncKind asyncKind, bool tryAnnexB, Directives inheritedDirectives,
Expand Down Expand Up @@ -2890,12 +2846,12 @@ bool Parser<FullParseHandler, Unit>::trySyntaxParseInnerFunction(
// still expects a FunctionBox to be attached to it during BCE, and
// the syntax parser cannot attach one to it.
FunctionBox* funbox =
newFunctionBox(*funNode, fcd, toStringStart, inheritedDirectives,
generatorKind, asyncKind);
newFunctionBox(*funNode, explicitName, flags, toStringStart,
inheritedDirectives, generatorKind, asyncKind);
if (!funbox) {
return false;
}
funbox->initWithEnclosingParseContext(pc_, fcd, kind);
funbox->initWithEnclosingParseContext(pc_, flags, kind);

SyntaxParseHandler::Node syntaxNode =
syntaxParser->innerFunctionForFunctionBox(
Expand Down Expand Up @@ -2934,9 +2890,10 @@ bool Parser<FullParseHandler, Unit>::trySyntaxParseInnerFunction(
} while (false);

// We failed to do a syntax parse above, so do the full parse.
FunctionNodeType innerFunc = innerFunction(
*funNode, pc_, fcd, toStringStart, inHandling, yieldHandling, kind,
generatorKind, asyncKind, tryAnnexB, inheritedDirectives, newDirectives);
FunctionNodeType innerFunc =
innerFunction(*funNode, pc_, explicitName, flags, toStringStart,
inHandling, yieldHandling, kind, generatorKind, asyncKind,
tryAnnexB, inheritedDirectives, newDirectives);
if (!innerFunc) {
return false;
}
Expand All @@ -2947,15 +2904,16 @@ bool Parser<FullParseHandler, Unit>::trySyntaxParseInnerFunction(

template <typename Unit>
bool Parser<SyntaxParseHandler, Unit>::trySyntaxParseInnerFunction(
FunctionNodeType* funNode, Handle<FunctionCreationData> fcd,
FunctionNodeType* funNode, HandleAtom explicitName, FunctionFlags flags,
uint32_t toStringStart, InHandling inHandling, YieldHandling yieldHandling,
FunctionSyntaxKind kind, GeneratorKind generatorKind,
FunctionAsyncKind asyncKind, bool tryAnnexB, Directives inheritedDirectives,
Directives* newDirectives) {
// This is already a syntax parser, so just parse the inner function.
FunctionNodeType innerFunc = innerFunction(
*funNode, pc_, fcd, toStringStart, inHandling, yieldHandling, kind,
generatorKind, asyncKind, tryAnnexB, inheritedDirectives, newDirectives);
FunctionNodeType innerFunc =
innerFunction(*funNode, pc_, explicitName, flags, toStringStart,
inHandling, yieldHandling, kind, generatorKind, asyncKind,
tryAnnexB, inheritedDirectives, newDirectives);

if (!innerFunc) {
return false;
Expand All @@ -2967,14 +2925,15 @@ bool Parser<SyntaxParseHandler, Unit>::trySyntaxParseInnerFunction(

template <class ParseHandler, typename Unit>
inline bool GeneralParser<ParseHandler, Unit>::trySyntaxParseInnerFunction(
FunctionNodeType* funNode, Handle<FunctionCreationData> fcd,
FunctionNodeType* funNode, HandleAtom explicitName, FunctionFlags flags,
uint32_t toStringStart, InHandling inHandling, YieldHandling yieldHandling,
FunctionSyntaxKind kind, GeneratorKind generatorKind,
FunctionAsyncKind asyncKind, bool tryAnnexB, Directives inheritedDirectives,
Directives* newDirectives) {
return asFinalParser()->trySyntaxParseInnerFunction(
funNode, fcd, toStringStart, inHandling, yieldHandling, kind,
generatorKind, asyncKind, tryAnnexB, inheritedDirectives, newDirectives);
funNode, explicitName, flags, toStringStart, inHandling, yieldHandling,
kind, generatorKind, asyncKind, tryAnnexB, inheritedDirectives,
newDirectives);
}

template <class ParseHandler, typename Unit>
Expand Down Expand Up @@ -3009,9 +2968,9 @@ GeneralParser<ParseHandler, Unit>::innerFunctionForFunctionBox(
template <class ParseHandler, typename Unit>
typename ParseHandler::FunctionNodeType
GeneralParser<ParseHandler, Unit>::innerFunction(
FunctionNodeType funNode, ParseContext* outerpc,
Handle<FunctionCreationData> fcd, uint32_t toStringStart,
InHandling inHandling, YieldHandling yieldHandling, FunctionSyntaxKind kind,
FunctionNodeType funNode, ParseContext* outerpc, HandleAtom explicitName,
FunctionFlags flags, uint32_t toStringStart, InHandling inHandling,
YieldHandling yieldHandling, FunctionSyntaxKind kind,
GeneratorKind generatorKind, FunctionAsyncKind asyncKind, bool tryAnnexB,
Directives inheritedDirectives, Directives* newDirectives) {
// Note that it is possible for outerpc != this->pc_, as we may be
Expand All @@ -3020,12 +2979,12 @@ GeneralParser<ParseHandler, Unit>::innerFunction(
// instead of the current top of the stack of the syntax parser.

FunctionBox* funbox =
newFunctionBox(funNode, fcd, toStringStart, inheritedDirectives,
generatorKind, asyncKind);
newFunctionBox(funNode, explicitName, flags, toStringStart,
inheritedDirectives, generatorKind, asyncKind);
if (!funbox) {
return null();
}
funbox->initWithEnclosingParseContext(outerpc, fcd, kind);
funbox->initWithEnclosingParseContext(outerpc, flags, kind);

FunctionNodeType innerFunc = innerFunctionForFunctionBox(
funNode, outerpc, funbox, inHandling, yieldHandling, kind, newDirectives);
Expand Down Expand Up @@ -7383,11 +7342,10 @@ GeneralParser<ParseHandler, Unit>::synthesizeConstructor(
? FunctionSyntaxKind::DerivedClassConstructor
: FunctionSyntaxKind::ClassConstructor;

Rooted<FunctionCreationData> data(
cx_, FunctionCreationData(
className, functionSyntaxKind, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction, options().selfHostingMode,
pc_->isFunctionBox()));
bool isSelfHosting = options().selfHostingMode;
FunctionFlags flags =
InitialFunctionFlags(functionSyntaxKind, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction, isSelfHosting);

// Create the top-level field initializer node.
FunctionNodeType funNode = handler_.newFunction(functionSyntaxKind, pos());
Expand All @@ -7397,13 +7355,13 @@ GeneralParser<ParseHandler, Unit>::synthesizeConstructor(

// Create the FunctionBox and link it to the function object.
Directives directives(true);
FunctionBox* funbox = newFunctionBox(funNode, data, classNameOffset,
directives, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction);
FunctionBox* funbox = newFunctionBox(
funNode, className, flags, classNameOffset, directives,
GeneratorKind::NotGenerator, FunctionAsyncKind::SyncFunction);
if (!funbox) {
return null();
}
funbox->initWithEnclosingParseContext(pc_, data, functionSyntaxKind);
funbox->initWithEnclosingParseContext(pc_, flags, functionSyntaxKind);
setFunctionEndFromCurrentToken(funbox);

// Push a SourceParseContext on to the stack.
Expand Down Expand Up @@ -7553,11 +7511,10 @@ GeneralParser<ParseHandler, Unit>::fieldInitializerOpt(Node propName,
firstTokenPos = TokenPos(endPos, endPos);
}

Rooted<FunctionCreationData> data(
cx_, FunctionCreationData(
nullptr, FunctionSyntaxKind::Method, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction, options().selfHostingMode,
pc_->isFunctionBox()));
bool isSelfHosting = options().selfHostingMode;
FunctionFlags flags = InitialFunctionFlags(
FunctionSyntaxKind::Method, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction, isSelfHosting);

// Create the top-level field initializer node.
FunctionNodeType funNode =
Expand All @@ -7568,13 +7525,13 @@ GeneralParser<ParseHandler, Unit>::fieldInitializerOpt(Node propName,

// Create the FunctionBox and link it to the function object.
Directives directives(true);
FunctionBox* funbox = newFunctionBox(funNode, data, firstTokenPos.begin,
directives, GeneratorKind::NotGenerator,
FunctionAsyncKind::SyncFunction);
FunctionBox* funbox = newFunctionBox(
funNode, nullptr, flags, firstTokenPos.begin, directives,
GeneratorKind::NotGenerator, FunctionAsyncKind::SyncFunction);
if (!funbox) {
return null();
}
funbox->initFieldInitializer(pc_, data);
funbox->initFieldInitializer(pc_, flags);

// We can't use setFunctionStartAtCurrentToken because that uses pos().begin,
// which is incorrect for fields without initializers (pos() points to the
Expand Down
Loading

0 comments on commit 067091a

Please sign in to comment.