Skip to content

Commit

Permalink
Correctly restore whether or not a function is an inner generator
Browse files Browse the repository at this point in the history
Summary:
If a generator was large enough to be lazily compiled, we would lose
that information when reconstituting the function's context. This meant
the function was generated as a regular function instead of a generator.

#utd-hermes-ignore-android

Reviewed By: tmikov

Differential Revision: D23580247

fbshipit-source-id: af5628bf322cbdc7c7cdfbb5f8d0756328518ea1
  • Loading branch information
willholen authored and facebook-github-bot committed Sep 8, 2020
1 parent cba0416 commit 0918353
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 21 deletions.
4 changes: 3 additions & 1 deletion include/hermes/IR/IR.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,10 @@ class SerializedScope {
#ifndef HERMESVM_LEAN
/// The source of a lazy AST node.
struct LazySource {
// The type of node (such as a FunctionDeclaration or FunctionExpression).
/// The type of node (such as a FunctionDeclaration or FunctionExpression).
ESTree::NodeKind nodeKind{ESTree::NodeKind::Empty};
/// Whether or not this is the inner function of a generator
bool isGeneratorInnerFunction;
/// The source buffer id in which this function can be find.
uint32_t bufferId{0};
/// The range of the function within the buffer (the whole function node, not
Expand Down
3 changes: 3 additions & 0 deletions include/hermes/IRGen/IRGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ struct LazyCompilationData {
/// The type of function, e.g. statement or expression.
ESTree::NodeKind nodeKind;

/// Whether or not this is the inner function of a generator.
bool isGeneratorInnerFunction;

/// Whether or not the function is strict.
bool strictMode;
};
Expand Down
2 changes: 2 additions & 0 deletions lib/BCGen/HBC/BytecodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ std::unique_ptr<BytecodeModule> BytecodeModuleGenerator::generate() {
auto lazyData = llvh::make_unique<LazyCompilationData>();
lazyData->parentScope = F->getLazyScope();
lazyData->nodeKind = F->getLazySource().nodeKind;
lazyData->isGeneratorInnerFunction =
F->getLazySource().isGeneratorInnerFunction;
lazyData->bufferId = F->getLazySource().bufferId;
lazyData->originalName = F->getOriginalOrInferredName();
lazyData->closureAlias = F->getLazyClosureAlias()
Expand Down
16 changes: 10 additions & 6 deletions lib/IRGen/ESTreeIRGen-func.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ Function *ESTreeIRGen::genES5Function(
auto &lazySource = newFunction->getLazySource();
lazySource.bufferId = bodyBlock->bufferId;
lazySource.nodeKind = getLazyFunctionKind(functionNode);
lazySource.isGeneratorInnerFunction = isGeneratorInnerFunction;
lazySource.functionRange = functionNode->getSourceRange();

// Set the function's .length.
Expand Down Expand Up @@ -302,14 +303,17 @@ Function *ESTreeIRGen::genGeneratorFunction(
ESTree::isStrict(functionNode->strictness),
/* insertBefore */ nullptr);

auto *innerFn = genES5Function(
genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
lazyClosureAlias,
functionNode,
true);

{
FunctionContext outerFnContext{this, outerFn, functionNode->getSemInfo()};

// Build the inner function. This must be done in the outerFnContext
// since it's lexically considered a child function.
auto *innerFn = genES5Function(
genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
lazyClosureAlias,
functionNode,
true);

emitFunctionPrologue(
functionNode,
Builder.createBasicBlock(outerFn),
Expand Down
6 changes: 5 additions & 1 deletion lib/IRGen/ESTreeIRGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,11 @@ std::pair<Function *, Function *> ESTreeIRGen::doLazyFunction(
!llvh::isa<ESTree::ArrowFunctionExpressionNode>(node) &&
"lazy compilation not supported for arrow functions");

auto *func = genES5Function(lazyData->originalName, parentVar, node);
auto *func = genES5Function(
lazyData->originalName,
parentVar,
node,
lazyData->isGeneratorInnerFunction);
addLexicalDebugInfo(func, topLevel, lexicalScopeChain);
return {func, topLevel};
}
Expand Down
2 changes: 1 addition & 1 deletion test/BCGen/HBC/es6/generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function *args() {
// CHECK-NEXT: CreateGenerator r1, r0, 4
// CHECK-NEXT: Ret r1

// CHECK-LABEL: Function<?anon_1_args>(1 params, 7 registers, 0 symbols):
// CHECK-LABEL: Function<?anon_0_args>(1 params, 7 registers, 0 symbols):
// CHECK-NEXT: Offset in debug table: source 0x{{.*}}, lexical 0x0000
// CHECK-NEXT: StartGenerator
// CHECK-NEXT: CreateEnvironment r0
Expand Down
24 changes: 12 additions & 12 deletions test/IRGen/es6/generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ function *useResult() {
//CHECK-NEXT:frame = [x]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StoreFrameInst undefined : undefined, [x]
//CHECK-NEXT: %1 = CreateGeneratorInst %?anon_1_useResult()
//CHECK-NEXT: %1 = CreateGeneratorInst %?anon_0_useResult()
//CHECK-NEXT: %2 = ReturnInst %1 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_1_useResult()
//CHECK-LABEL:function ?anon_0_useResult()
//CHECK-NEXT:frame = [x]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down Expand Up @@ -86,11 +86,11 @@ function *loop(x) {
//CHECK-NEXT:frame = [i]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StoreFrameInst undefined : undefined, [i]
//CHECK-NEXT: %1 = CreateGeneratorInst %?anon_2_loop()
//CHECK-NEXT: %1 = CreateGeneratorInst %?anon_0_loop()
//CHECK-NEXT: %2 = ReturnInst %1 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_2_loop(x)
//CHECK-LABEL:function ?anon_0_loop(x)
//CHECK-NEXT:frame = [i, x]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down Expand Up @@ -139,11 +139,11 @@ var simple2 = function*() {
//CHECK-LABEL:function simple2()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_4_simple2()
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_0_simple2()
//CHECK-NEXT: %1 = ReturnInst %0 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_4_simple2()
//CHECK-LABEL:function ?anon_0_simple2()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down Expand Up @@ -172,11 +172,11 @@ var yieldStar = function*() {
//CHECK-LABEL:function yieldStar()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_5_yieldStar()
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_0_yieldStar()
//CHECK-NEXT: %1 = ReturnInst %0 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_5_yieldStar()
//CHECK-LABEL:function ?anon_0_yieldStar()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down Expand Up @@ -281,13 +281,13 @@ var destr = function*([x]) {
//CHECK-LABEL:function destr()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_6_destr()
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_0_destr()
//CHECK-NEXT: %1 = LoadPropertyInst %0 : object, "next" : string
//CHECK-NEXT: %2 = CallInst %1, %0 : object
//CHECK-NEXT: %3 = ReturnInst %0 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_6_destr(?anon_2_param)
//CHECK-LABEL:function ?anon_0_destr(?anon_2_param)
//CHECK-NEXT:frame = [x]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down Expand Up @@ -355,13 +355,13 @@ var initializer = function*(x = foo()) {
//CHECK-LABEL:function initializer()
//CHECK-NEXT:frame = []
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_7_initializer()
//CHECK-NEXT: %0 = CreateGeneratorInst %?anon_0_initializer()
//CHECK-NEXT: %1 = LoadPropertyInst %0 : object, "next" : string
//CHECK-NEXT: %2 = CallInst %1, %0 : object
//CHECK-NEXT: %3 = ReturnInst %0 : object
//CHECK-NEXT:function_end

//CHECK-LABEL:function ?anon_7_initializer(x)
//CHECK-LABEL:function ?anon_0_initializer(x)
//CHECK-NEXT:frame = [x]
//CHECK-NEXT:%BB0:
//CHECK-NEXT: %0 = StartGeneratorInst
Expand Down
28 changes: 28 additions & 0 deletions test/IRGen/lazy-function-in-generator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// RUN: %hermes -lazy %s | %FileCheck %s --match-full-lines

// Make sure we can correctly resolve scopes through lazily compiled
// functions in lazily compiled generators.
function f() {
var f_var = 10;
function* g() {
var g_var = 32;
function h() {
/* Some text to pad out the function so that it won't be eagerly compiled
* for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing
* elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
*/
return f_var + g_var;
}
yield h();
}
return g().next().value;
}
// CHECK: 42
print(f());

0 comments on commit 0918353

Please sign in to comment.