Skip to content

Commit

Permalink
Bug 1520996 - Use AccessorType instead of JSOP for getters/setters. r…
Browse files Browse the repository at this point in the history 
…=arai

This will take a very slight memory regression until JSOP is entirely
removed from ParseNode (which will regain a *lot* more memory).

Differential Revision: https://phabricator.services.mozilla.com/D24293
  • Loading branch information
@khyperia
khyperia committed Mar 21, 2019
1 parent 0fe0f49 commit d5ad266
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 82 deletions.
2 changes: 1 addition & 1 deletion js/src/builtin/ModuleObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ bool ModuleBuilder::processExportObjectBinding(frontend::ListNode* obj) {

for (ParseNode* node : obj->contents()) {
MOZ_ASSERT(node->isKind(ParseNodeKind::MutateProto) ||
node->isKind(ParseNodeKind::Colon) ||
node->isKind(ParseNodeKind::PropertyDefinition) ||
node->isKind(ParseNodeKind::Shorthand) ||
node->isKind(ParseNodeKind::Spread));

Expand Down
43 changes: 25 additions & 18 deletions js/src/builtin/ReflectParse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2500,16 +2500,16 @@ bool ASTSerializer::statement(ParseNode* pn, MutableHandleValue dst) {
bool ASTSerializer::classMethod(ClassMethod* classMethod,
MutableHandleValue dst) {
PropKind kind;
switch (classMethod->getOp()) {
case JSOP_INITPROP:
switch (classMethod->accessorType()) {
case AccessorType::None:
kind = PROP_INIT;
break;

case JSOP_INITPROP_GETTER:
case AccessorType::Getter:
kind = PROP_GETTER;
break;

case JSOP_INITPROP_SETTER:
case AccessorType::Setter:
kind = PROP_SETTER;
break;

Expand Down Expand Up @@ -3113,21 +3113,26 @@ bool ASTSerializer::property(ParseNode* pn, MutableHandleValue dst) {
}

PropKind kind;
switch (pn->getOp()) {
case JSOP_INITPROP:
kind = PROP_INIT;
break;
if (pn->is<PropertyDefinition>()) {
switch (pn->as<PropertyDefinition>().accessorType()) {
case AccessorType::None:
kind = PROP_INIT;
break;

case JSOP_INITPROP_GETTER:
kind = PROP_GETTER;
break;
case AccessorType::Getter:
kind = PROP_GETTER;
break;

case JSOP_INITPROP_SETTER:
kind = PROP_SETTER;
break;
case AccessorType::Setter:
kind = PROP_SETTER;
break;

default:
LOCAL_NOT_REACHED("unexpected object-literal property");
default:
LOCAL_NOT_REACHED("unexpected object-literal property");
}
} else {
MOZ_ASSERT(pn->isKind(ParseNodeKind::Shorthand));
kind = PROP_INIT;
}

BinaryNode* node = &pn->as<BinaryNode>();
Expand Down Expand Up @@ -3251,8 +3256,10 @@ bool ASTSerializer::objectPattern(ListNode* obj, MutableHandleValue dst) {
elts.infallibleAppend(spread);
continue;
}
LOCAL_ASSERT(propdef->isKind(ParseNodeKind::MutateProto) !=
propdef->isOp(JSOP_INITPROP));
// Patterns can't have getters/setters.
LOCAL_ASSERT(!propdef->is<PropertyDefinition>() ||
propdef->as<PropertyDefinition>().accessorType() ==
AccessorType::None);

RootedValue key(cx);
ParseNode* target;
Expand Down
51 changes: 28 additions & 23 deletions js/src/frontend/BytecodeEmitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ bool BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer) {
*answer = true;
return true;

case ParseNodeKind::Colon:
case ParseNodeKind::PropertyDefinition:
case ParseNodeKind::Case: {
BinaryNode* node = &pn->as<BinaryNode>();
if (!checkSideEffects(node->left(), answer)) {
Expand Down Expand Up @@ -3620,7 +3620,7 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern,
member->isKind(ParseNodeKind::Spread)) {
subpattern = member->as<UnaryNode>().kid();
} else {
MOZ_ASSERT(member->isKind(ParseNodeKind::Colon) ||
MOZ_ASSERT(member->isKind(ParseNodeKind::PropertyDefinition) ||
member->isKind(ParseNodeKind::Shorthand));
subpattern = member->as<BinaryNode>().right();
}
Expand Down Expand Up @@ -3698,7 +3698,7 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern,
}
needsGetElem = false;
} else {
MOZ_ASSERT(member->isKind(ParseNodeKind::Colon) ||
MOZ_ASSERT(member->isKind(ParseNodeKind::PropertyDefinition) ||
member->isKind(ParseNodeKind::Shorthand));

ParseNode* key = member->as<BinaryNode>().left();
Expand Down Expand Up @@ -7569,16 +7569,21 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,

ParseNode* key = prop->left();
ParseNode* propVal = prop->right();
JSOp op = propdef->getOp();
MOZ_ASSERT(op == JSOP_INITPROP || op == JSOP_INITPROP_GETTER ||
op == JSOP_INITPROP_SETTER);
AccessorType accessorType;
if (prop->is<ClassMethod>()) {
accessorType = prop->as<ClassMethod>().accessorType();
} else if (prop->is<PropertyDefinition>()) {
accessorType = prop->as<PropertyDefinition>().accessorType();
} else {
accessorType = AccessorType::None;
}

auto emitValue = [this, &key, &propVal, op, &pe]() {
auto emitValue = [this, &key, &propVal, accessorType, &pe]() {
// [stack] CTOR? OBJ CTOR? KEY?

if (propVal->isDirectRHSAnonFunction()) {
if (key->isKind(ParseNodeKind::NumberExpr)) {
MOZ_ASSERT(op == JSOP_INITPROP);
MOZ_ASSERT(accessorType == AccessorType::None);

NumericLiteral* literal = &key->as<NumericLiteral>();
RootedAtom keyAtom(cx, NumberToAtom(cx, literal->value()));
Expand All @@ -7591,7 +7596,7 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
}
} else if (key->isKind(ParseNodeKind::ObjectPropertyName) ||
key->isKind(ParseNodeKind::StringExpr)) {
MOZ_ASSERT(op == JSOP_INITPROP);
MOZ_ASSERT(accessorType == AccessorType::None);

RootedAtom keyAtom(cx, key->as<NameNode>().atom());
if (!emitAnonymousFunctionWithName(propVal, keyAtom)) {
Expand All @@ -7601,9 +7606,9 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
} else {
MOZ_ASSERT(key->isKind(ParseNodeKind::ComputedName));

FunctionPrefixKind prefix = op == JSOP_INITPROP
FunctionPrefixKind prefix = accessorType == AccessorType::None
? FunctionPrefixKind::None
: op == JSOP_INITPROP_GETTER
: accessorType == AccessorType::Getter
? FunctionPrefixKind::Get
: FunctionPrefixKind::Set;

Expand Down Expand Up @@ -7657,20 +7662,20 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
return false;
}

switch (op) {
case JSOP_INITPROP:
switch (accessorType) {
case AccessorType::None:
if (!pe.emitInitIndexProp()) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_GETTER:
case AccessorType::Getter:
if (!pe.emitInitIndexGetter()) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_SETTER:
case AccessorType::Setter:
if (!pe.emitInitIndexSetter()) {
// [stack] CTOR? OBJ
return false;
Expand Down Expand Up @@ -7704,20 +7709,20 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
}

RootedAtom keyAtom(cx, key->as<NameNode>().atom());
switch (op) {
case JSOP_INITPROP:
switch (accessorType) {
case AccessorType::None:
if (!pe.emitInitProp(keyAtom)) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_GETTER:
case AccessorType::Getter:
if (!pe.emitInitGetter(keyAtom)) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_SETTER:
case AccessorType::Setter:
if (!pe.emitInitSetter(keyAtom)) {
// [stack] CTOR? OBJ
return false;
Expand Down Expand Up @@ -7751,20 +7756,20 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
return false;
}

switch (op) {
case JSOP_INITPROP:
switch (accessorType) {
case AccessorType::None:
if (!pe.emitInitComputedProp()) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_GETTER:
case AccessorType::Getter:
if (!pe.emitInitComputedGetter()) {
// [stack] CTOR? OBJ
return false;
}
break;
case JSOP_INITPROP_SETTER:
case AccessorType::Setter:
if (!pe.emitInitComputedSetter()) {
// [stack] CTOR? OBJ
return false;
Expand Down
4 changes: 2 additions & 2 deletions js/src/frontend/FoldConstants.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static bool ContainsHoistedDeclaration(JSContext* cx, ParseNode* node,
case ParseNodeKind::ComputedName:
case ParseNodeKind::Spread:
case ParseNodeKind::MutateProto:
case ParseNodeKind::Colon:
case ParseNodeKind::PropertyDefinition:
case ParseNodeKind::Shorthand:
case ParseNodeKind::ConditionalExpr:
case ParseNodeKind::TypeOfNameExpr:
Expand Down Expand Up @@ -1491,7 +1491,7 @@ class FoldVisitor : public RewritingParseNodeVisitor<FoldVisitor> {
ListNode* list = &pn->as<ListNode>();
if (list->hasNonConstInitializer()) {
for (ParseNode* node : list->contents()) {
if (node->getKind() != ParseNodeKind::Colon) {
if (node->getKind() != ParseNodeKind::PropertyDefinition) {
return true;
}
BinaryNode* binary = &node->as<BinaryNode>();
Expand Down
10 changes: 4 additions & 6 deletions js/src/frontend/FullParseHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,12 @@ class FullParseHandler {
BinaryNodeType newPropertyDefinition(Node key, Node val) {
MOZ_ASSERT(isUsableAsObjectPropertyName(key));
checkAndSetIsDirectRHSAnonFunction(val);
return newBinary(ParseNodeKind::Colon, key, val, JSOP_INITPROP);
return new_<PropertyDefinition>(key, val, AccessorType::None);
}

void addPropertyDefinition(ListNodeType literal, BinaryNodeType propdef) {
MOZ_ASSERT(literal->isKind(ParseNodeKind::ObjectExpr));
MOZ_ASSERT(propdef->isKind(ParseNodeKind::Colon));
MOZ_ASSERT(propdef->isKind(ParseNodeKind::PropertyDefinition));

if (!propdef->right()->isConstant()) {
literal->setHasNonConstInitializer();
Expand Down Expand Up @@ -451,8 +451,7 @@ class FullParseHandler {

checkAndSetIsDirectRHSAnonFunction(funNode);

ClassMethod* classMethod =
new_<ClassMethod>(key, funNode, AccessorTypeToJSOp(atype), isStatic);
ClassMethod* classMethod = new_<ClassMethod>(key, funNode, atype, isStatic);
if (!classMethod) {
return false;
}
Expand Down Expand Up @@ -780,8 +779,7 @@ class FullParseHandler {
AccessorType atype) {
MOZ_ASSERT(isUsableAsObjectPropertyName(key));

return newBinary(ParseNodeKind::Colon, key, value,
AccessorTypeToJSOp(atype));
return new_<PropertyDefinition>(key, value, atype);
}

BinaryNodeType newShorthandPropertyDefinition(Node key, Node value) {
Expand Down
9 changes: 4 additions & 5 deletions js/src/frontend/NameFunctions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,10 @@ class NameResolver : public ParseNodeVisitor<NameResolver> {
}
break;

case ParseNodeKind::Colon:
case ParseNodeKind::PropertyDefinition:
case ParseNodeKind::Shorthand:
// Record the ParseNodeKind::Colon/Shorthand but skip the
// ParseNodeKind::Object so we're not flagged as a
// contributor.
// Record the ParseNodeKind::PropertyDefinition/Shorthand but skip the
// ParseNodeKind::Object so we're not flagged as a contributor.
pos--;
MOZ_FALLTHROUGH;

Expand Down Expand Up @@ -272,7 +271,7 @@ class NameResolver : public ParseNodeVisitor<NameResolver> {
for (int pos = size - 1; pos >= 0; pos--) {
ParseNode* node = toName[pos];

if (node->isKind(ParseNodeKind::Colon) ||
if (node->isKind(ParseNodeKind::PropertyDefinition) ||
node->isKind(ParseNodeKind::Shorthand)) {
ParseNode* left = node->as<BinaryNode>().left();
if (left->isKind(ParseNodeKind::ObjectPropertyName) ||
Expand Down
Loading

0 comments on commit d5ad266

Please sign in to comment.