Skip to content

Commit 5fa1ffe

Browse files
boingoingboingoing
committed
[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
2 parents 773e5f9 + d8b4a23 commit 5fa1ffe

File tree

6 files changed

+149
-25
lines changed

6 files changed

+149
-25
lines changed

‎lib/Parser/Parse.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7128,6 +7128,8 @@ template<bool buildAST>
71287128
ParseNodePtr Parser::GenerateModuleFunctionWrapper()
71297129
{
71307130
ParseNodePtr pnodeFnc = ParseFncDeclNoCheckScope<buildAST>(fFncModule, SuperRestrictionState::Disallowed, nullptr, /* needsPIDOnRCurlyScan */ false, /* fUnaryOrParen */ true);
7131+
// mark modules as generators after parsing - this is to enable cross-module hoisting of exported functions
7132+
pnodeFnc->AsParseNodeFnc()->SetIsGenerator(true);
71317133
ParseNodePtr callNode = CreateCallNode(knopCall, pnodeFnc, nullptr);
71327134

71337135
return callNode;

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ void EmitAssignment(ParseNode *asgnNode, ParseNode *lhs, Js::RegSlot rhsLocation
1212
void EmitLoad(ParseNode *rhs, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
1313
void EmitCall(ParseNodeCall* pnodeCall, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, BOOL fEvaluateComponents, Js::RegSlot overrideThisLocation = Js::Constants::NoRegister, Js::RegSlot newTargetLocation = Js::Constants::NoRegister);
1414
void EmitYield(Js::RegSlot inputLocation, Js::RegSlot resultLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, bool isAsync = false, bool isAwait = false, Js::RegSlot yieldStarIterator = Js::Constants::NoRegister);
15+
void EmitDummyYield(ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo);
1516

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

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

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

3078+
// insert a yield at the top of a module body so that function definitions can be hoisted accross modules
3079+
if (pnodeFnc->IsModule())
3080+
{
3081+
EmitDummyYield(this, funcInfo);
3082+
}
3083+
30803084
if (pnodeFnc->HasNonSimpleParameterList() || !funcInfo->IsBodyAndParamScopeMerged())
30813085
{
30823086
Assert(pnodeFnc->HasNonSimpleParameterList() || CONFIG_FLAG(ForceSplitScope));
@@ -10241,6 +10245,18 @@ void ByteCodeGenerator::EmitTryBlockHeadersAfterYield()
1024110245
}
1024210246
}
1024310247

10248+
void EmitDummyYield(ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo)
10249+
{
10250+
byteCodeGenerator->EmitLeaveOpCodesBeforeYield();
10251+
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, funcInfo->yieldRegister);
10252+
byteCodeGenerator->Writer()->Reg2(Js::OpCode::Yield, funcInfo->yieldRegister, funcInfo->yieldRegister);
10253+
byteCodeGenerator->EmitTryBlockHeadersAfterYield();
10254+
Js::RegSlot unusedResult = funcInfo->AcquireTmpRegister();
10255+
byteCodeGenerator->Writer()->Reg2(Js::OpCode::ResumeYield, unusedResult, funcInfo->yieldRegister);
10256+
byteCodeGenerator->Writer()->Reg1(Js::OpCode::Unused, unusedResult);
10257+
funcInfo->ReleaseTmpRegister(unusedResult);
10258+
}
10259+
1024410260
void EmitYield(Js::RegSlot inputLocation, Js::RegSlot resultLocation, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, bool isAsync, bool isAwait, Js::RegSlot yieldStarIterator)
1024510261
{
1024610262
// If the bytecode emitted by this function is part of 'yield*', inputLocation is the object

‎lib/Runtime/Language/SourceTextModuleRecord.cpp

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -971,15 +971,8 @@ namespace Js
971971
}
972972
}
973973

974-
Var SourceTextModuleRecord::ModuleEvaluation()
974+
bool SourceTextModuleRecord::ModuleEvaluationPrepass()
975975
{
976-
OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ModuleEvaluation(%s)\n"), this->GetSpecifierSz());
977-
978-
if (!scriptContext->GetConfig()->IsES6ModuleEnabled() || WasEvaluated())
979-
{
980-
return nullptr;
981-
}
982-
983976
if (this->errorObject != nullptr)
984977
{
985978
// Cleanup in case of error.
@@ -988,13 +981,14 @@ namespace Js
988981
if (this->promise != nullptr)
989982
{
990983
SourceTextModuleRecord::ResolveOrRejectDynamicImportPromise(false, this->errorObject, this->scriptContext, this, false);
991-
return scriptContext->GetLibrary()->GetUndefined();
984+
return false;
992985
}
993986
else
994987
{
995988
JavascriptExceptionOperators::Throw(errorObject, this->scriptContext);
996989
}
997990
}
991+
SetEvaluationPrepassed();
998992

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

1004+
JavascriptExceptionObject *exception = nullptr;
1005+
1006+
try
1007+
{
1008+
if (childrenModuleSet != nullptr)
1009+
{
1010+
childrenModuleSet->EachValue([=](SourceTextModuleRecord* childModuleRecord)
1011+
{
1012+
if (!childModuleRecord->WasEvaluationPrepassed())
1013+
{
1014+
childModuleRecord->ModuleEvaluationPrepass();
1015+
}
1016+
1017+
// if child module was evaluated before and threw need to re-throw now
1018+
// if child module has been dynamically imported and has exception need to throw
1019+
if (childModuleRecord->GetErrorObject() != nullptr)
1020+
{
1021+
this->ReleaseParserResourcesForHierarchy();
1022+
1023+
JavascriptExceptionOperators::Throw(childModuleRecord->GetErrorObject(), this->scriptContext);
1024+
}
1025+
});
1026+
}
1027+
1028+
AUTO_NESTED_HANDLED_EXCEPTION_TYPE((ExceptionType)(ExceptionType_OutOfMemory | ExceptionType_JavascriptException));
1029+
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
1030+
{
1031+
Arguments outArgs(CallInfo(CallFlags_Value, 0), nullptr);
1032+
this->generator = VarTo<JavascriptGenerator>(rootFunction->CallRootFunction(outArgs, scriptContext, true));
1033+
}
1034+
END_SAFE_REENTRANT_CALL
1035+
}
1036+
catch (const Js::JavascriptException &err)
1037+
{
1038+
exception = err.GetAndClear();
1039+
Var errorObject = exception->GetThrownObject(scriptContext);
1040+
AssertOrFailFastMsg(errorObject != nullptr, "ModuleEvaluation: null error object thrown from root function");
1041+
this->errorObject = errorObject;
1042+
if (this->promise != nullptr)
1043+
{
1044+
ResolveOrRejectDynamicImportPromise(false, errorObject, scriptContext, this, false);
1045+
return false;
1046+
}
1047+
}
1048+
1049+
if (exception != nullptr)
1050+
{
1051+
JavascriptExceptionOperators::DoThrowCheckClone(exception, scriptContext);
1052+
}
1053+
return true;
1054+
}
1055+
1056+
1057+
Var SourceTextModuleRecord::ModuleEvaluation()
1058+
{
1059+
OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("ModuleEvaluation(%s)\n"), this->GetSpecifierSz());
1060+
1061+
if (!scriptContext->GetConfig()->IsES6ModuleEnabled() || WasEvaluated())
1062+
{
1063+
return nullptr;
1064+
}
1065+
1066+
if (!WasEvaluationPrepassed())
1067+
{
1068+
if (!ModuleEvaluationPrepass())
1069+
{
1070+
return scriptContext->GetLibrary()->GetUndefined();
1071+
}
1072+
}
1073+
10101074
Assert(this->errorObject == nullptr);
10111075
SetWasEvaluated();
10121076

@@ -1019,10 +1083,7 @@ namespace Js
10191083
{
10201084
childrenModuleSet->EachValue([=](SourceTextModuleRecord* childModuleRecord)
10211085
{
1022-
if (!childModuleRecord->WasEvaluated())
1023-
{
1024-
childModuleRecord->ModuleEvaluation();
1025-
}
1086+
childModuleRecord->ModuleEvaluation();
10261087
// if child module was evaluated before and threw need to re-throw now
10271088
// if child module has been dynamically imported and has exception need to throw
10281089
if (childModuleRecord->GetErrorObject() != nullptr)
@@ -1035,13 +1096,15 @@ namespace Js
10351096
}
10361097
CleanupBeforeExecution();
10371098

1038-
Arguments outArgs(CallInfo(CallFlags_Value, 0), nullptr);
1099+
JavascriptGenerator* gen = static_cast<JavascriptGenerator*> (generator);
10391100

10401101
AUTO_NESTED_HANDLED_EXCEPTION_TYPE((ExceptionType)(ExceptionType_OutOfMemory | ExceptionType_JavascriptException));
1041-
ENTER_SCRIPT_IF(scriptContext, true, false, false, !scriptContext->GetThreadContext()->IsScriptActive(),
1102+
BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
10421103
{
1043-
ret = rootFunction->CallRootFunction(outArgs, scriptContext, true);
1044-
});
1104+
ResumeYieldData yieldData(scriptContext->GetLibrary()->GetUndefined(), nullptr);
1105+
ret = gen->CallGenerator(&yieldData, _u("Module Global"));
1106+
}
1107+
END_SAFE_REENTRANT_CALL
10451108
}
10461109
catch (const Js::JavascriptException &err)
10471110
{

‎lib/Runtime/Language/SourceTextModuleRecord.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ namespace Js
3535
bool ModuleDeclarationInstantiation() override;
3636
void GenerateRootFunction();
3737
Var ModuleEvaluation() override;
38+
bool ModuleEvaluationPrepass();
3839
virtual ModuleNamespace* GetNamespace();
3940
virtual void SetNamespace(ModuleNamespace* moduleNamespace);
4041

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

6263
bool WasParsed() const { return wasParsed; }
6364
void SetWasParsed() { wasParsed = true; }
65+
bool WasEvaluationPrepassed() const { return wasPrepassed; }
66+
void SetEvaluationPrepassed() { wasPrepassed = true; }
6467
bool WasDeclarationInitialized() const { return wasDeclarationInitialized; }
6568
void SetWasDeclarationInitialized() { wasDeclarationInitialized = true; }
6669
void SetIsRootModule() { isRootModule = true; }
@@ -122,11 +125,13 @@ namespace Js
122125
// This is the parsed tree resulted from compilation.
123126
Field(bool) confirmedReady = false;
124127
Field(bool) notifying = false;
128+
Field(bool) wasPrepassed = false;
125129
Field(bool) wasParsed;
126130
Field(bool) wasDeclarationInitialized;
127131
Field(bool) parentsNotified;
128132
Field(bool) isRootModule;
129133
Field(bool) hadNotifyHostReady;
134+
Field(JavascriptGenerator*) generator;
130135
Field(ParseNodeProg *) parseTree;
131136
Field(Utf8SourceInfo*) pSourceInfo;
132137
Field(uint) sourceIndex;

‎lib/Runtime/Library/JavascriptGenerator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ namespace Js
5757
}
5858
}
5959

60+
Var CallGenerator(ResumeYieldData* yieldData, const char16* apiNameForErrorMessage);
61+
6062
private:
6163
Field(InterpreterStackFrame*) frame;
6264
Field(GeneratorState) state;
@@ -74,7 +76,6 @@ namespace Js
7476
DEFINE_VTABLE_CTOR_MEMBER_INIT(JavascriptGenerator, DynamicObject, args);
7577
DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptGenerator);
7678

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

8081
public:

‎test/es6module/module-bugfixes.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,43 @@ var tests = [
130130

131131
testRunner.LoadModule('import "test6133a";', undefined, true);
132132
}
133+
},
134+
{
135+
name : "Issue 5236: Module function exports not hoisted test",
136+
body()
137+
{
138+
WScript.RegisterModuleSource("test5236a", `
139+
import {default as Default, bar, foo} from "test5236b";
140+
export default function () {}
141+
export function one() {}
142+
export var two = function () {}
143+
144+
bar();
145+
assert.isNotUndefined(Default);
146+
foo();
147+
`);
148+
WScript.RegisterModuleSource("test5236b", `
149+
import Default from "test5236c";
150+
151+
export function bar() {}
152+
export default class {}
153+
export var foo = function () {}
154+
155+
Default();
156+
`);
157+
WScript.RegisterModuleSource("test5236c", `
158+
import {default as Default, one, two} from "test5236a";
159+
import otherDefault from "test5236b";
160+
export default function bar() {}
161+
162+
Default();
163+
one();
164+
assert.isUndefined(two);
165+
assert.isUndefined(otherDefault);
166+
`);
167+
168+
testRunner.LoadModule('import "test5236a";', undefined, false);
169+
}
133170
}
134171
];
135172

0 commit comments

Comments
 (0)