Skip to content
Permalink
Browse files

[MERGE #6171 @rhuanjl] Hoist Module Function Exports

Merge pull request #6171 from rhuanjl:hoistExportedFunctions

This PR enables function exports to be hoisted across modules as per  https://tc39.es/ecma262/#sec-source-text-module-record-initialize-environment

I believe this is the last significant bug in the module implementation (there remain two issues with not getting the right errors thrown but those don't impact correct code)

This is done by following @guybedford's advice of converting module root functions into generator functions with a yield at the top after functions are defined they are then executed in two steps so that all functions are declared first.

**Side effects of this change:**
1. The bottom line of the stack trace for errors thrown in modules  `at module` which was in all module error stack traces has been removed - could probably revert this but saw no value to the line
2. Any loops in the module global will now NOT be eligible for jitting - as JitLoopBody is disabled in generator functions - this is probably fixable for modules with some special casing but not currently sure which conditions to edit

@boingoing please could you take a look at this?

fixes: #5236
  • Loading branch information...
boingoing committed Jul 3, 2019
2 parents 773e5f9 + d8b4a23 commit 5fa1ffeec1aeec0a0de7c823deaabd910426c3cc
@@ -7128,6 +7128,8 @@ template<bool buildAST>
ParseNodePtr Parser::GenerateModuleFunctionWrapper()
{
ParseNodePtr pnodeFnc = ParseFncDeclNoCheckScope<buildAST>(fFncModule, SuperRestrictionState::Disallowed, nullptr, /* needsPIDOnRCurlyScan */ false, /* fUnaryOrParen */ true);
// mark modules as generators after parsing - this is to enable cross-module hoisting of exported functions
pnodeFnc->AsParseNodeFnc()->SetIsGenerator(true);
ParseNodePtr callNode = CreateCallNode(knopCall, pnodeFnc, nullptr);

return callNode;
@@ -12,6 +12,7 @@ void EmitAssignment(ParseNode *asgnNode, ParseNode *lhs, Js::RegSlot rhsLocation
void EmitLoad(ParseNode *rhs, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
void EmitCall(ParseNodeCall* pnodeCall, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, BOOL fEvaluateComponents, Js::RegSlot overrideThisLocation = Js::Constants::NoRegister, Js::RegSlot newTargetLocation = Js::Constants::NoRegister);
void EmitYield(Js::RegSlot inputLocation, Js::RegSlot resultLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, bool isAsync = false, bool isAwait = false, Js::RegSlot yieldStarIterator = Js::Constants::NoRegister);
void EmitDummyYield(ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo);

void EmitUseBeforeDeclaration(Symbol *sym, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
void EmitUseBeforeDeclarationRuntimeError(ByteCodeGenerator *byteCodeGenerator, Js::RegSlot location);
@@ -1079,7 +1080,7 @@ void ByteCodeGenerator::DefineCachedFunctions(FuncInfo *funcInfoParent)
auto fillEntries = [&](ParseNode *pnodeFnc)
{
Symbol *sym = pnodeFnc->AsParseNodeFnc()->GetFuncSymbol();
if (sym != nullptr && (pnodeFnc->AsParseNodeFnc()->IsDeclaration()))
if (sym != nullptr && (pnodeFnc->AsParseNodeFnc()->IsDeclaration() || pnodeFnc->AsParseNodeFnc()->IsDefaultModuleExport()))
{
AssertMsg(!pnodeFnc->AsParseNodeFnc()->IsGenerator(), "Generator functions are not supported by InitCachedFuncs but since they always escape they should disable function caching");
Js::FuncInfoEntry *entry = &info->elements[slotCount];
@@ -1146,7 +1147,7 @@ void ByteCodeGenerator::DefineUncachedFunctions(FuncInfo *funcInfoParent)
// after the assignment. Might save register.
//

if (pnodeFnc->AsParseNodeFnc()->IsDeclaration())
if (pnodeFnc->AsParseNodeFnc()->IsDeclaration() || pnodeFnc->AsParseNodeFnc()->IsDefaultModuleExport())
{
this->DefineOneFunction(pnodeFnc->AsParseNodeFnc(), funcInfoParent);
// The "x = function() {...}" case is being generated on the fly, during emission,
@@ -3056,12 +3057,9 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc)
// (that is when the function is called). This yield opcode is to mark the begining of the function body.
// TODO: Inserting a yield should have almost no impact on perf as it is a direct return from the function. But this needs
// to be verified. Ideally if the function has simple parameter list then we can avoid inserting the opcode and the additional call.
if (pnodeFnc->IsGenerator())
if (pnodeFnc->IsGenerator() && !pnodeFnc->IsModule())
{
Js::RegSlot tempReg = funcInfo->AcquireTmpRegister();
EmitYield(funcInfo->AssignUndefinedConstRegister(), tempReg, this, funcInfo);
m_writer.Reg1(Js::OpCode::Unused, tempReg);
funcInfo->ReleaseTmpRegister(tempReg);
EmitDummyYield(this, funcInfo);
}

DefineUserVars(funcInfo);
@@ -3077,6 +3075,12 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc)
DefineFunctions(funcInfo);
}

// insert a yield at the top of a module body so that function definitions can be hoisted accross modules
if (pnodeFnc->IsModule())
{
EmitDummyYield(this, funcInfo);
}

if (pnodeFnc->HasNonSimpleParameterList() || !funcInfo->IsBodyAndParamScopeMerged())
{
Assert(pnodeFnc->HasNonSimpleParameterList() || CONFIG_FLAG(ForceSplitScope));
@@ -10241,6 +10245,18 @@ void ByteCodeGenerator::EmitTryBlockHeadersAfterYield()
}
}

void EmitDummyYield(ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo)
{
byteCodeGenerator->EmitLeaveOpCodesBeforeYield();
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, funcInfo->yieldRegister);
byteCodeGenerator->Writer()->Reg2(Js::OpCode::Yield, funcInfo->yieldRegister, funcInfo->yieldRegister);
byteCodeGenerator->EmitTryBlockHeadersAfterYield();
Js::RegSlot unusedResult = funcInfo->AcquireTmpRegister();
byteCodeGenerator->Writer()->Reg2(Js::OpCode::ResumeYield, unusedResult, funcInfo->yieldRegister);
byteCodeGenerator->Writer()->Reg1(Js::OpCode::Unused, unusedResult);
funcInfo->ReleaseTmpRegister(unusedResult);
}

void EmitYield(Js::RegSlot inputLocation, Js::RegSlot resultLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, bool isAsync, bool isAwait, Js::RegSlot yieldStarIterator)
{
// If the bytecode emitted by this function is part of 'yield*', inputLocation is the object
@@ -971,15 +971,8 @@ namespace Js
}
}

Var SourceTextModuleRecord::ModuleEvaluation()
bool SourceTextModuleRecord::ModuleEvaluationPrepass()
{
OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ModuleEvaluation(%s)\n"), this->GetSpecifierSz());

if (!scriptContext->GetConfig()->IsES6ModuleEnabled() || WasEvaluated())
{
return nullptr;
}

if (this->errorObject != nullptr)
{
// Cleanup in case of error.
@@ -988,13 +981,14 @@ namespace Js
if (this->promise != nullptr)
{
SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this, false);
return scriptContext->GetLibrary()->GetUndefined();
return false;
}
else
{
JavascriptExceptionOperators::Throw(errorObject, this->scriptContext);
}
}
SetEvaluationPrepassed();

#if DBG
if (childrenModuleSet != nullptr)
@@ -1007,6 +1001,76 @@ namespace Js
}
#endif

JavascriptExceptionObject *exception = nullptr;

try
{
if (childrenModuleSet != nullptr)
{
childrenModuleSet->EachValue([=](SourceTextModuleRecord* childModuleRecord)
{
if (!childModuleRecord->WasEvaluationPrepassed())
{
childModuleRecord->ModuleEvaluationPrepass();
}

// if child module was evaluated before and threw need to re-throw now
// if child module has been dynamically imported and has exception need to throw
if (childModuleRecord->GetErrorObject() != nullptr)
{
this->ReleaseParserResourcesForHierarchy();

JavascriptExceptionOperators::Throw(childModuleRecord->GetErrorObject(), this->scriptContext);
}
});
}

AUTO_NESTED_HANDLED_EXCEPTION_TYPE((ExceptionType)(ExceptionType_OutOfMemory | ExceptionType_JavascriptException));
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
{
Arguments outArgs(CallInfo(CallFlags_Value, 0), nullptr);
this->generator = VarTo<JavascriptGenerator>(rootFunction->CallRootFunction(outArgs, scriptContext, true));
}
END_SAFE_REENTRANT_CALL
}
catch (const Js::JavascriptException &err)
{
exception = err.GetAndClear();
Var errorObject = exception->GetThrownObject(scriptContext);
AssertOrFailFastMsg(errorObject != nullptr, "ModuleEvaluation: null error object thrown from root function");
this->errorObject = errorObject;
if (this->promise != nullptr)
{
ResolveOrRejectDynamicImportPromise(false, errorObject, scriptContext, this, false);
return false;
}
}

if (exception != nullptr)
{
JavascriptExceptionOperators::DoThrowCheckClone(exception, scriptContext);
}
return true;
}


Var SourceTextModuleRecord::ModuleEvaluation()
{
OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ModuleEvaluation(%s)\n"), this->GetSpecifierSz());

if (!scriptContext->GetConfig()->IsES6ModuleEnabled() || WasEvaluated())
{
return nullptr;
}

if (!WasEvaluationPrepassed())
{
if (!ModuleEvaluationPrepass())
{
return scriptContext->GetLibrary()->GetUndefined();
}
}

Assert(this->errorObject == nullptr);
SetWasEvaluated();

@@ -1019,10 +1083,7 @@ namespace Js
{
childrenModuleSet->EachValue([=](SourceTextModuleRecord* childModuleRecord)
{
if (!childModuleRecord->WasEvaluated())
{
childModuleRecord->ModuleEvaluation();
}
childModuleRecord->ModuleEvaluation();
// if child module was evaluated before and threw need to re-throw now
// if child module has been dynamically imported and has exception need to throw
if (childModuleRecord->GetErrorObject() != nullptr)
@@ -1035,13 +1096,15 @@ namespace Js
}
CleanupBeforeExecution();

Arguments outArgs(CallInfo(CallFlags_Value, 0), nullptr);
JavascriptGenerator* gen = static_cast<JavascriptGenerator*> (generator);

AUTO_NESTED_HANDLED_EXCEPTION_TYPE((ExceptionType)(ExceptionType_OutOfMemory | ExceptionType_JavascriptException));
ENTER_SCRIPT_IF(scriptContext, true, false, false, !scriptContext->GetThreadContext()->IsScriptActive(),
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
{
ret = rootFunction->CallRootFunction(outArgs, scriptContext, true);
});
ResumeYieldData yieldData(scriptContext->GetLibrary()->GetUndefined(), nullptr);
ret = gen->CallGenerator(&yieldData, _u("Module Global"));
}
END_SAFE_REENTRANT_CALL
}
catch (const Js::JavascriptException &err)
{
@@ -35,6 +35,7 @@ namespace Js
bool ModuleDeclarationInstantiation() override;
void GenerateRootFunction();
Var ModuleEvaluation() override;
bool ModuleEvaluationPrepass();
virtual ModuleNamespace* GetNamespace();
virtual void SetNamespace(ModuleNamespace* moduleNamespace);

@@ -61,6 +62,8 @@ namespace Js

bool WasParsed() const { return wasParsed; }
void SetWasParsed() { wasParsed = true; }
bool WasEvaluationPrepassed() const { return wasPrepassed; }
void SetEvaluationPrepassed() { wasPrepassed = true; }
bool WasDeclarationInitialized() const { return wasDeclarationInitialized; }
void SetWasDeclarationInitialized() { wasDeclarationInitialized = true; }
void SetIsRootModule() { isRootModule = true; }
@@ -122,11 +125,13 @@ namespace Js
// This is the parsed tree resulted from compilation.
Field(bool) confirmedReady = false;
Field(bool) notifying = false;
Field(bool) wasPrepassed = false;
Field(bool) wasParsed;
Field(bool) wasDeclarationInitialized;
Field(bool) parentsNotified;
Field(bool) isRootModule;
Field(bool) hadNotifyHostReady;
Field(JavascriptGenerator*) generator;
Field(ParseNodeProg *) parseTree;
Field(Utf8SourceInfo*) pSourceInfo;
Field(uint) sourceIndex;
@@ -57,6 +57,8 @@ namespace Js
}
}

Var CallGenerator(ResumeYieldData* yieldData, const char16* apiNameForErrorMessage);

private:
Field(InterpreterStackFrame*) frame;
Field(GeneratorState) state;
@@ -74,7 +76,6 @@ namespace Js
DEFINE_VTABLE_CTOR_MEMBER_INIT(JavascriptGenerator, DynamicObject, args);
DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptGenerator);

Var CallGenerator(ResumeYieldData* yieldData, const char16* apiNameForErrorMessage);
JavascriptGenerator(DynamicType* type, Arguments& args, ScriptFunction* scriptFunction);

public:
@@ -130,6 +130,43 @@ var tests = [

testRunner.LoadModule('import "test6133a";', undefined, true);
}
},
{
name : "Issue 5236: Module function exports not hoisted test",
body()
{
WScript.RegisterModuleSource("test5236a", `
import {default as Default, bar, foo} from "test5236b";
export default function () {}
export function one() {}
export var two = function () {}
bar();
assert.isNotUndefined(Default);
foo();
`);
WScript.RegisterModuleSource("test5236b", `
import Default from "test5236c";
export function bar() {}
export default class {}
export var foo = function () {}
Default();
`);
WScript.RegisterModuleSource("test5236c", `
import {default as Default, one, two} from "test5236a";
import otherDefault from "test5236b";
export default function bar() {}
Default();
one();
assert.isUndefined(two);
assert.isUndefined(otherDefault);
`);

testRunner.LoadModule('import "test5236a";', undefined, false);
}
}
];

0 comments on commit 5fa1ffe

Please sign in to comment.
You can’t perform that action at this time.