Skip to content

Commit

Permalink
Add Function::DefinitionKind::ES6DerivedConstructor
Browse files Browse the repository at this point in the history
Summary:
Add a new ES6DerivedConstructor kind to DefinitionKind. Audit all
existing usages of what is now `ES6BaseConstructor`.

Reviewed By: tmikov

Differential Revision: D67215064

fbshipit-source-id: 3e26ca9624f560c232a80ac01a7a840470860041
  • Loading branch information
fbmal7 authored and facebook-github-bot committed Dec 18, 2024
1 parent 9a78f63 commit bb1c7be
Show file tree
Hide file tree
Showing 19 changed files with 68 additions and 59 deletions.
3 changes: 2 additions & 1 deletion include/hermes/IR/IR.h
Original file line number Diff line number Diff line change
Expand Up @@ -1773,7 +1773,8 @@ class Function : public llvh::ilist_node_with_parent<Function, Module>,

enum class DefinitionKind {
ES5Function,
ES6Constructor,
ES6BaseConstructor,
ES6DerivedConstructor,
ES6Arrow,
ES6Method,
// This corresponds to the synthetic function we create in IRGen, which is
Expand Down
12 changes: 8 additions & 4 deletions lib/IR/IR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ static Type functionNewTargetType(Function::DefinitionKind defKind) {
switch (defKind) {
case Function::DefinitionKind::ES5Function:
return Type::unionTy(Type::createObject(), Type::createUndefined());
case Function::DefinitionKind::ES6Constructor:
case Function::DefinitionKind::ES6BaseConstructor:
case Function::DefinitionKind::ES6DerivedConstructor:
return Type::createObject();
case Function::DefinitionKind::ES6Arrow:
case Function::DefinitionKind::GeneratorInnerArrow:
Expand Down Expand Up @@ -229,8 +230,10 @@ std::string Function::getDefinitionKindStr(bool isDescriptive) const {
switch (definitionKind_) {
case Function::DefinitionKind::ES5Function:
return "function";
case Function::DefinitionKind::ES6Constructor:
return "constructor";
case Function::DefinitionKind::ES6BaseConstructor:
return "base constructor";
case Function::DefinitionKind::ES6DerivedConstructor:
return "derived constructor";
case Function::DefinitionKind::ES6Arrow:
return isDescriptive ? "arrow function" : "arrow";
case Function::DefinitionKind::ES6Method:
Expand Down Expand Up @@ -283,7 +286,8 @@ llvh::Optional<llvh::StringRef> Function::getSourceRepresentationStr() const {

Function::ProhibitInvoke Function::getProhibitInvoke() const {
// ES6 constructors must be invoked as constructors.
if (definitionKind_ == DefinitionKind::ES6Constructor)
if (definitionKind_ == DefinitionKind::ES6BaseConstructor ||
definitionKind_ == DefinitionKind::ES6DerivedConstructor)
return ProhibitInvoke::ProhibitCall;

// Generators, async functions, methods, and arrow functions may not be
Expand Down
3 changes: 2 additions & 1 deletion lib/IR/IRVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,8 @@ bool Verifier::visitGetNewTargetInst(GetNewTargetInst const &Inst) {
AssertIWithMsg(
Inst,
definitionKind == Function::DefinitionKind::ES5Function ||
definitionKind == Function::DefinitionKind::ES6Constructor ||
definitionKind == Function::DefinitionKind::ES6BaseConstructor ||
definitionKind == Function::DefinitionKind::ES6DerivedConstructor ||
definitionKind == Function::DefinitionKind::ES6Method,
"GetNewTargetInst can only be used in ES6 constructors, ES5 functions, and ES6 methods");
AssertIWithMsg(
Expand Down
3 changes: 2 additions & 1 deletion lib/IRGen/ESTreeIRGen-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2476,7 +2476,8 @@ Value *ESTreeIRGen::genNewTarget() {

switch (curFunction()->function->getDefinitionKind()) {
case Function::DefinitionKind::ES5Function:
case Function::DefinitionKind::ES6Constructor:
case Function::DefinitionKind::ES6BaseConstructor:
case Function::DefinitionKind::ES6DerivedConstructor:
value = Builder.createGetNewTargetInst(
curFunction()->function->getNewTargetParam());
break;
Expand Down
54 changes: 26 additions & 28 deletions lib/IRGen/ESTreeIRGen-func.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,33 +403,33 @@ NormalFunction *ESTreeIRGen::genBasicFunction(
parentScope);
}

if (functionKind == Function::DefinitionKind::ES6Constructor) {
if (curFunction()->hasLegacyClassContext()) {
if (curFunction()->getSemInfo()->constructorKind ==
sema::FunctionInfo::ConstructorKind::Derived) {
// Initialize the 'checked this' in derived class constructors.
newFunctionContext.capturedState.thisVal = Builder.createVariable(
curFunction()->curScope->getVariableScope(),
Builder.createIdentifier("?CHECKED_this"),
Type::unionTy(Type::createObject(), Type::createEmpty()),
true);
Builder.createStoreFrameInst(
curFunction()->curScope,
Builder.getLiteralEmpty(),
newFunctionContext.capturedState.thisVal);
} else {
// We generate this call after calling super for derived classes.
emitLegacyInstanceElementsInitCall();
}
} else {
if (curFunction()->hasLegacyClassContext()) {
if (functionKind == Function::DefinitionKind::ES6BaseConstructor) {
// We only need to generate this here for base classes. It's not
// required for derived because they generate this call after calling
// super().
emitLegacyInstanceElementsInitCall();
} else if (
functionKind == Function::DefinitionKind::ES6DerivedConstructor) {
// Initialize the 'checked this' in derived class constructors.
newFunctionContext.capturedState.thisVal = Builder.createVariable(
curFunction()->curScope->getVariableScope(),
Builder.createIdentifier("?CHECKED_this"),
Type::unionTy(Type::createObject(), Type::createEmpty()),
true);
Builder.createStoreFrameInst(
curFunction()->curScope,
Builder.getLiteralEmpty(),
newFunctionContext.capturedState.thisVal);
}
} else {
if (functionKind == Function::DefinitionKind::ES6BaseConstructor) {
assert(
curFunction()->hasTypedClassContext() &&
"ES6Constructor has no valid class context");
// If we're compiling a typed constructor with no superclass, emit the
// field inits at the start.
if (superClassNode == nullptr) {
emitTypedFieldInitCall(typedClassContext.type);
}
// If we're compiling a typed base class constructor, emit the field
// inits at the start.
emitTypedFieldInitCall(typedClassContext.type);
}
}

Expand Down Expand Up @@ -701,10 +701,8 @@ void ESTreeIRGen::initCaptureStateInES5FunctionHelper() {
// `this` is managed separately in the case of a legacy derived class
// constructor.
if (!(curFunction()->function->getDefinitionKind() ==
Function::DefinitionKind::ES6Constructor &&
curFunction()->hasLegacyClassContext() &&
semCtx_.nearestNonArrow(curFunction()->getSemInfo())->constructorKind ==
sema::FunctionInfo::ConstructorKind::Derived)) {
Function::DefinitionKind::ES6DerivedConstructor &&
curFunction()->hasLegacyClassContext())) {
auto *th = Builder.createVariable(
scope,
genAnonymousLabelName("this"),
Expand Down
7 changes: 5 additions & 2 deletions lib/IRGen/ESTreeIRGen-legacy-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ CreateClassInst *ESTreeIRGen::genLegacyClassLike(
llvh::cast<ESTree::FunctionExpressionNode>(consMethodNode->_value),
curScope->getVariableScope(),
superClassNode,
Function::DefinitionKind::ES6Constructor,
superClassNode ? Function::DefinitionKind::ES6DerivedConstructor
: Function::DefinitionKind::ES6BaseConstructor,
clsPrototypeVar,
consMethodNode);
}
Expand Down Expand Up @@ -385,7 +386,9 @@ NormalFunction *ESTreeIRGen::genLegacyImplicitConstructor(

auto *consFunc = Builder.createFunction(
className,
Function::DefinitionKind::ES6Constructor,
funcInfo->constructorKind == sema::FunctionInfo::ConstructorKind::Derived
? Function::DefinitionKind::ES6DerivedConstructor
: Function::DefinitionKind::ES6BaseConstructor,
/* strictMode */ true,
funcInfo->customDirectives);

Expand Down
3 changes: 2 additions & 1 deletion lib/IRGen/ESTreeIRGen-typed-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ void ESTreeIRGen::genClassDeclaration(ESTree::ClassDeclarationNode *node) {
llvh::cast<ESTree::FunctionExpressionNode>(consMethod->_value),
consName,
node->_superClass,
Function::DefinitionKind::ES6Constructor);
node->_superClass ? Function::DefinitionKind::ES6DerivedConstructor
: Function::DefinitionKind::ES6BaseConstructor);
} else {
// The constructor is implicit.
consFunction = genTypedImplicitConstructor(consName, superClass);
Expand Down
4 changes: 2 additions & 2 deletions test/IRGen/flow/class-constructor-super.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ new D();

// CHECK:scope %VS2 []

// CHECK:constructor C(): any [typed]
// CHECK:base constructor C(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateScopeInst (:environment) %VS2: any, %0: environment
Expand All @@ -82,7 +82,7 @@ new D();

// CHECK:scope %VS3 []

// CHECK:constructor D(): any [typed]
// CHECK:derived constructor D(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
2 changes: 1 addition & 1 deletion test/IRGen/flow/class-method-inherited.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ new D().inherited();

// CHECK:scope %VS4 []

// CHECK:constructor D(): any [typed]
// CHECK:derived constructor D(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
2 changes: 1 addition & 1 deletion test/IRGen/flow/class-method-override.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function foo(c: C, d: D){

// CHECK:scope %VS6 []

// CHECK:constructor D(): any [typed]
// CHECK:derived constructor D(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
4 changes: 2 additions & 2 deletions test/IRGen/flow/class-super-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class B extends A {

// CHECK:scope %VS2 [x: any]

// CHECK:constructor A(x: number): any [typed]
// CHECK:base constructor A(x: number): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down Expand Up @@ -107,7 +107,7 @@ class B extends A {

// CHECK:scope %VS4 [x: any]

// CHECK:constructor B(x: number): any [typed]
// CHECK:derived constructor B(x: number): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
4 changes: 2 additions & 2 deletions test/IRGen/flow/class-super-prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class B extends A {

// CHECK:scope %VS2 [x: any]

// CHECK:constructor A(x: number): any [typed]
// CHECK:base constructor A(x: number): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand All @@ -89,7 +89,7 @@ class B extends A {

// CHECK:scope %VS3 [x: any]

// CHECK:constructor B(x: number): any [typed]
// CHECK:derived constructor B(x: number): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
2 changes: 1 addition & 1 deletion test/IRGen/flow/class1.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ return [dotProduct, Vec2D];
// CHECK-NEXT: ReturnInst %8: number
// CHECK-NEXT:function_end

// CHECK:constructor Vec2D(x: number, y: number): undefined [typed]
// CHECK:base constructor Vec2D(x: number, y: number): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = LoadParamInst (:number) %y: number
Expand Down
8 changes: 4 additions & 4 deletions test/IRGen/flow/field-inits.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ function f(i: number): number {

// CHECK:scope %VS4 []

// CHECK:constructor A(): any [typed]
// CHECK:base constructor A(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down Expand Up @@ -194,7 +194,7 @@ function f(i: number): number {

// CHECK:scope %VS6 []

// CHECK:constructor B(): any [typed]
// CHECK:base constructor B(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down Expand Up @@ -259,7 +259,7 @@ function f(i: number): number {

// CHECK:scope %VS11 []

// CHECK:constructor C1(): any [typed]
// CHECK:derived constructor C1(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down Expand Up @@ -290,7 +290,7 @@ function f(i: number): number {

// CHECK:scope %VS13 []

// CHECK:constructor "A 1#"(): any [typed]
// CHECK:base constructor "A 1#"(): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS2: any, %parentScope: environment
Expand Down
4 changes: 2 additions & 2 deletions test/IRGen/flow/generic-class-id.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const s: string = i2.val;

// CHECK:scope %VS2 [val: any]

// CHECK:constructor ID(val: number): any [typed]
// CHECK:base constructor ID(val: number): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand All @@ -94,7 +94,7 @@ const s: string = i2.val;

// CHECK:scope %VS3 [val: any]

// CHECK:constructor "ID 1#"(val: string): any [typed]
// CHECK:base constructor "ID 1#"(val: string): any [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
2 changes: 1 addition & 1 deletion test/Optimizer/flow/inline-constructor-small.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ print(p1.x, p2.x);
// CHECK-NEXT: ReturnInst undefined: undefined
// CHECK-NEXT:function_end

// CHECK:constructor Point(x: number, y: number): undefined [typed]
// CHECK:base constructor Point(x: number, y: number): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = LoadParamInst (:number) %x: number
Expand Down
4 changes: 2 additions & 2 deletions test/Optimizer/object-with-object-field-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ new Foo();
// CHECK-NEXT: ReturnInst %12: undefined
// CHECK-NEXT:function_end

// CHECK:constructor O(): undefined [typed]
// CHECK:base constructor O(): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = HBCLoadConstInst (:number) 7: number
// CHECK-NEXT: %1 = LoadParamInst (:object) %<this>: object
Expand All @@ -60,7 +60,7 @@ new Foo();
// CHECK-NEXT: ReturnInst %3: undefined
// CHECK-NEXT:function_end

// CHECK:constructor Foo(): undefined [typed]
// CHECK:base constructor Foo(): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down
2 changes: 1 addition & 1 deletion test/Optimizer/typed-object-promotion.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ print(foo.x, foo.y);
// CHECK-NEXT: ReturnInst undefined: undefined
// CHECK-NEXT:function_end

// CHECK:constructor Foo(x: number, y: number): undefined [typed]
// CHECK:base constructor Foo(x: number, y: number): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = LoadParamInst (:number) %x: number
Expand Down
4 changes: 2 additions & 2 deletions test/Optimizer/uninit-prop-access-opt.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ print(f());
// CHECK-NEXT: ReturnInst undefined: undefined
// CHECK-NEXT:function_end

// CHECK:constructor O(): undefined [typed]
// CHECK:base constructor O(): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: PrStoreInst 7: number, %0: object, 0: number, "i": string, true: boolean
// CHECK-NEXT: ReturnInst undefined: undefined
// CHECK-NEXT:function_end

// CHECK:constructor Foo(): undefined [typed]
// CHECK:base constructor Foo(): undefined [typed]
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = LoadParamInst (:object) %<this>: object
// CHECK-NEXT: %1 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
Expand Down

0 comments on commit bb1c7be

Please sign in to comment.