[JSC] Make Operator an enum class to avoid Op* identifiers
authorross.kirsling@sony.com <ross.kirsling@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Mar 2020 02:47:07 +0000 (02:47 +0000)
committerross.kirsling@sony.com <ross.kirsling@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Mar 2020 02:47:07 +0000 (02:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209637

Reviewed by Darin Adler.

Currently, (e.g.) OpLShift is a value of enum Operator while OpLshift is an opcode.
Capitalization aside, it's confusing to be using Op* for disparate purposes like this.
Let's modernize the enum so that this confusion can go away as a side effect.

* bytecompiler/NodesCodegen.cpp:
(JSC::emitIncOrDec):
(JSC::PostfixNode::emitBytecode):
(JSC::PrefixNode::emitBytecode):
(JSC::LogicalOpNode::emitBytecode):
(JSC::LogicalOpNode::emitBytecodeInConditionContext):
(JSC::emitReadModifyAssignment):
(JSC::ReadModifyDotNode::emitBytecode):
(JSC::ReadModifyBracketNode::emitBytecode):
* parser/ASTBuilder.h:
(JSC::ASTBuilder::makeBinaryNode):
(JSC::ASTBuilder::makeAssignNode):
* parser/Nodes.h:
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseAssignmentExpression):
(JSC::Parser<LexerType>::parseUnaryExpression):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@259150 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/parser/ASTBuilder.h
Source/JavaScriptCore/parser/Nodes.h
Source/JavaScriptCore/parser/Parser.cpp

index d94850b..bc79288 100644 (file)
@@ -1,3 +1,31 @@
+2020-03-27  Ross Kirsling  <ross.kirsling@sony.com>
+
+        [JSC] Make Operator an enum class to avoid Op* identifiers
+        https://bugs.webkit.org/show_bug.cgi?id=209637
+
+        Reviewed by Darin Adler.
+
+        Currently, (e.g.) OpLShift is a value of enum Operator while OpLshift is an opcode.
+        Capitalization aside, it's confusing to be using Op* for disparate purposes like this.
+        Let's modernize the enum so that this confusion can go away as a side effect.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::emitIncOrDec):
+        (JSC::PostfixNode::emitBytecode):
+        (JSC::PrefixNode::emitBytecode):
+        (JSC::LogicalOpNode::emitBytecode):
+        (JSC::LogicalOpNode::emitBytecodeInConditionContext):
+        (JSC::emitReadModifyAssignment):
+        (JSC::ReadModifyDotNode::emitBytecode):
+        (JSC::ReadModifyBracketNode::emitBytecode):
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::makeBinaryNode):
+        (JSC::ASTBuilder::makeAssignNode):
+        * parser/Nodes.h:
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseAssignmentExpression):
+        (JSC::Parser<LexerType>::parseUnaryExpression):
+
 2020-03-19  Tadeu Zagallo  <tzagallo@apple.com>
 
         Fix instances of new.target that should be syntax errors
index d14c306..640b249 100644 (file)
@@ -1879,7 +1879,7 @@ RegisterID* ApplyFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator,
 
 static RegisterID* emitIncOrDec(BytecodeGenerator& generator, RegisterID* srcDst, Operator oper)
 {
-    return (oper == OpPlusPlus) ? generator.emitInc(srcDst) : generator.emitDec(srcDst);
+    return (oper == Operator::PlusPlus) ? generator.emitInc(srcDst) : generator.emitDec(srcDst);
 }
 
 static RegisterID* emitPostIncOrDec(BytecodeGenerator& generator, RegisterID* dst, RegisterID* srcDst, Operator oper)
@@ -2008,7 +2008,7 @@ RegisterID* PostfixNode::emitBytecode(BytecodeGenerator& generator, RegisterID*
         return emitDot(generator, dst);
 
     ASSERT(m_expr->isFunctionCall());
-    return emitThrowReferenceError(generator, m_operator == OpPlusPlus
+    return emitThrowReferenceError(generator, m_operator == Operator::PlusPlus
         ? "Postfix ++ operator applied to value that is not a reference."_s
         : "Postfix -- operator applied to value that is not a reference."_s);
 }
@@ -2230,7 +2230,7 @@ RegisterID* PrefixNode::emitBytecode(BytecodeGenerator& generator, RegisterID* d
         return emitDot(generator, dst);
 
     ASSERT(m_expr->isFunctionCall());
-    return emitThrowReferenceError(generator, m_operator == OpPlusPlus
+    return emitThrowReferenceError(generator, m_operator == Operator::PlusPlus
         ? "Prefix ++ operator applied to value that is not a reference."_s
         : "Prefix -- operator applied to value that is not a reference."_s);
 }
@@ -2662,7 +2662,7 @@ RegisterID* LogicalOpNode::emitBytecode(BytecodeGenerator& generator, RegisterID
     Ref<Label> target = generator.newLabel();
     
     generator.emitNode(temp.get(), m_expr1);
-    if (m_operator == OpLogicalAnd)
+    if (m_operator == LogicalOperator::And)
         generator.emitJumpIfFalse(temp.get(), target.get());
     else
         generator.emitJumpIfTrue(temp.get(), target.get());
@@ -2678,7 +2678,7 @@ void LogicalOpNode::emitBytecodeInConditionContext(BytecodeGenerator& generator,
         generator.emitDebugHook(this);
 
     Ref<Label> afterExpr1 = generator.newLabel();
-    if (m_operator == OpLogicalAnd)
+    if (m_operator == LogicalOperator::And)
         generator.emitNodeInConditionContext(m_expr1, afterExpr1.get(), falseTarget, FallThroughMeansTrue);
     else 
         generator.emitNodeInConditionContext(m_expr1, trueTarget, afterExpr1.get(), FallThroughMeansFalse);
@@ -2756,47 +2756,47 @@ static ALWAYS_INLINE RegisterID* emitReadModifyAssignment(BytecodeGenerator& gen
 {
     OpcodeID opcodeID;
     switch (oper) {
-        case OpMultEq:
-            opcodeID = op_mul;
-            break;
-        case OpDivEq:
-            opcodeID = op_div;
-            break;
-        case OpPlusEq:
-            if (m_right->isAdd() && m_right->resultDescriptor().definitelyIsString())
-                return static_cast<AddNode*>(m_right)->emitStrcat(generator, dst, src1, emitExpressionInfoForMe);
-            opcodeID = op_add;
-            break;
-        case OpMinusEq:
-            opcodeID = op_sub;
-            break;
-        case OpLShift:
-            opcodeID = op_lshift;
-            break;
-        case OpRShift:
-            opcodeID = op_rshift;
-            break;
-        case OpURShift:
-            opcodeID = op_urshift;
-            break;
-        case OpBitAndEq:
-            opcodeID = op_bitand;
-            break;
-        case OpBitXOrEq:
-            opcodeID = op_bitxor;
-            break;
-        case OpBitOrEq:
-            opcodeID = op_bitor;
-            break;
-        case OpModEq:
-            opcodeID = op_mod;
-            break;
-        case OpPowEq:
-            opcodeID = op_pow;
-            break;
-        default:
-            RELEASE_ASSERT_NOT_REACHED();
-            return dst;
+    case Operator::MultEq:
+        opcodeID = op_mul;
+        break;
+    case Operator::DivEq:
+        opcodeID = op_div;
+        break;
+    case Operator::PlusEq:
+        if (m_right->isAdd() && m_right->resultDescriptor().definitelyIsString())
+            return static_cast<AddNode*>(m_right)->emitStrcat(generator, dst, src1, emitExpressionInfoForMe);
+        opcodeID = op_add;
+        break;
+    case Operator::MinusEq:
+        opcodeID = op_sub;
+        break;
+    case Operator::LShift:
+        opcodeID = op_lshift;
+        break;
+    case Operator::RShift:
+        opcodeID = op_rshift;
+        break;
+    case Operator::URShift:
+        opcodeID = op_urshift;
+        break;
+    case Operator::BitAndEq:
+        opcodeID = op_bitand;
+        break;
+    case Operator::BitXOrEq:
+        opcodeID = op_bitxor;
+        break;
+    case Operator::BitOrEq:
+        opcodeID = op_bitor;
+        break;
+    case Operator::ModEq:
+        opcodeID = op_mod;
+        break;
+    case Operator::PowEq:
+        opcodeID = op_pow;
+        break;
+    default:
+        RELEASE_ASSERT_NOT_REACHED();
+        return dst;
     }
 
     RegisterID* src2 = generator.emitNode(m_right);
@@ -2806,7 +2806,7 @@ static ALWAYS_INLINE RegisterID* emitReadModifyAssignment(BytecodeGenerator& gen
     if (emitExpressionInfoForMe)
         generator.emitExpressionInfo(emitExpressionInfoForMe->divot(), emitExpressionInfoForMe->divotStart(), emitExpressionInfoForMe->divotEnd());
     RegisterID* result = generator.emitBinaryOp(opcodeID, dst, src1, src2, types);
-    if (oper == OpURShift)
+    if (oper == Operator::URShift)
         return generator.emitUnaryOp<OpUnsigned>(result, result);
     return result;
 }
@@ -2961,7 +2961,7 @@ RegisterID* ReadModifyDotNode::emitBytecode(BytecodeGenerator& generator, Regist
         value = generator.emitGetById(generator.tempDestination(dst), base.get(), thisValue.get(), m_ident);
     } else
         value = generator.emitGetById(generator.tempDestination(dst), base.get(), m_ident);
-    RegisterID* updatedValue = emitReadModifyAssignment(generator, generator.finalDestination(dst, value.get()), value.get(), m_right, static_cast<JSC::Operator>(m_operator), OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
+    RegisterID* updatedValue = emitReadModifyAssignment(generator, generator.finalDestination(dst, value.get()), value.get(), m_right, m_operator, OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
 
     generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
     RegisterID* ret;
@@ -3025,7 +3025,7 @@ RegisterID* ReadModifyBracketNode::emitBytecode(BytecodeGenerator& generator, Re
         value = generator.emitGetByVal(generator.tempDestination(dst), base.get(), thisValue.get(), property.get());
     } else
         value = generator.emitGetByVal(generator.tempDestination(dst), base.get(), property.get());
-    RegisterID* updatedValue = emitReadModifyAssignment(generator, generator.finalDestination(dst, value.get()), value.get(), m_right, static_cast<JSC::Operator>(m_operator), OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
+    RegisterID* updatedValue = emitReadModifyAssignment(generator, generator.finalDestination(dst, value.get()), value.get(), m_right, m_operator, OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
 
     generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
     if (m_base->isSuperNode())
index 4c61730..43b171f 100644 (file)
@@ -1458,10 +1458,10 @@ ExpressionNode* ASTBuilder::makeBinaryNode(const JSTokenLocation& location, int
         return makeCoalesceNode(location, lhs.first, rhs.first);
 
     case OR:
-        return new (m_parserArena) LogicalOpNode(location, lhs.first, rhs.first, OpLogicalOr);
+        return new (m_parserArena) LogicalOpNode(location, lhs.first, rhs.first, LogicalOperator::Or);
 
     case AND:
-        return new (m_parserArena) LogicalOpNode(location, lhs.first, rhs.first, OpLogicalAnd);
+        return new (m_parserArena) LogicalOpNode(location, lhs.first, rhs.first, LogicalOperator::And);
 
     case BITOR:
         return makeBitOrNode(location, lhs.first, rhs.first, rhs.second.hasAssignment);
@@ -1548,7 +1548,7 @@ ExpressionNode* ASTBuilder::makeAssignNode(const JSTokenLocation& location, Expr
 
     if (loc->isResolveNode()) {
         ResolveNode* resolve = static_cast<ResolveNode*>(loc);
-        if (op == OpEqual) {
+        if (op == Operator::Equal) {
             if (expr->isBaseFuncExprNode()) {
                 auto metadata = static_cast<BaseFuncExprNode*>(expr)->metadata();
                 metadata->setEcmaName(resolve->identifier());
@@ -1562,7 +1562,7 @@ ExpressionNode* ASTBuilder::makeAssignNode(const JSTokenLocation& location, Expr
     }
     if (loc->isBracketAccessorNode()) {
         BracketAccessorNode* bracket = static_cast<BracketAccessorNode*>(loc);
-        if (op == OpEqual)
+        if (op == Operator::Equal)
             return new (m_parserArena) AssignBracketNode(location, bracket->base(), bracket->subscript(), expr, locHasAssignments, exprHasAssignments, bracket->divot(), start, end);
         ReadModifyBracketNode* node = new (m_parserArena) ReadModifyBracketNode(location, bracket->base(), bracket->subscript(), op, expr, locHasAssignments, exprHasAssignments, divot, start, end);
         node->setSubexpressionInfo(bracket->divot(), bracket->divotEnd().offset);
@@ -1570,7 +1570,7 @@ ExpressionNode* ASTBuilder::makeAssignNode(const JSTokenLocation& location, Expr
     }
     ASSERT(loc->isDotAccessorNode());
     DotAccessorNode* dot = static_cast<DotAccessorNode*>(loc);
-    if (op == OpEqual)
+    if (op == Operator::Equal)
         return new (m_parserArena) AssignDotNode(location, dot->base(), dot->identifier(), expr, exprHasAssignments, dot->divot(), start, end);
 
     ReadModifyDotNode* node = new (m_parserArena) ReadModifyDotNode(location, dot->base(), dot->identifier(), op, expr, exprHasAssignments, divot, start, end);
index e3cf4bf..22df5ae 100644 (file)
@@ -55,27 +55,27 @@ namespace JSC {
 
     typedef SmallPtrSet<UniquedStringImpl*> UniquedStringImplPtrSet;
 
-    enum Operator : uint8_t {
-        OpEqual,
-        OpPlusEq,
-        OpMinusEq,
-        OpMultEq,
-        OpDivEq,
-        OpPlusPlus,
-        OpMinusMinus,
-        OpBitAndEq,
-        OpBitXOrEq,
-        OpBitOrEq,
-        OpModEq,
-        OpPowEq,
-        OpLShift,
-        OpRShift,
-        OpURShift
+    enum class Operator : uint8_t {
+        Equal,
+        PlusEq,
+        MinusEq,
+        MultEq,
+        DivEq,
+        PlusPlus,
+        MinusMinus,
+        BitAndEq,
+        BitXOrEq,
+        BitOrEq,
+        ModEq,
+        PowEq,
+        LShift,
+        RShift,
+        URShift
     };
     
-    enum LogicalOperator : uint8_t {
-        OpLogicalAnd,
-        OpLogicalOr
+    enum class LogicalOperator : uint8_t {
+        And,
+        Or
     };
 
     enum FallThroughMode : uint8_t {
@@ -1413,7 +1413,7 @@ namespace JSC {
         ExpressionNode* m_base;
         ExpressionNode* m_subscript;
         ExpressionNode* m_right;
-        unsigned m_operator : 30;
+        Operator m_operator;
         bool m_subscriptHasAssignments : 1;
         bool m_rightHasAssignments : 1;
     };
@@ -1455,7 +1455,7 @@ namespace JSC {
         ExpressionNode* m_base;
         const Identifier& m_ident;
         ExpressionNode* m_right;
-        unsigned m_operator : 31;
+        Operator m_operator;
         bool m_rightHasAssignments : 1;
     };
 
index 979b5db..73e83f3 100644 (file)
@@ -3844,19 +3844,19 @@ template <typename TreeBuilder> TreeExpression Parser<LexerType>::parseAssignmen
     bool hadAssignment = false;
     while (true) {
         switch (m_token.m_type) {
-        case EQUAL: op = OpEqual; break;
-        case PLUSEQUAL: op = OpPlusEq; break;
-        case MINUSEQUAL: op = OpMinusEq; break;
-        case MULTEQUAL: op = OpMultEq; break;
-        case DIVEQUAL: op = OpDivEq; break;
-        case LSHIFTEQUAL: op = OpLShift; break;
-        case RSHIFTEQUAL: op = OpRShift; break;
-        case URSHIFTEQUAL: op = OpURShift; break;
-        case BITANDEQUAL: op = OpBitAndEq; break;
-        case BITXOREQUAL: op = OpBitXOrEq; break;
-        case BITOREQUAL: op = OpBitOrEq; break;
-        case MODEQUAL: op = OpModEq; break;
-        case POWEQUAL: op = OpPowEq; break;
+        case EQUAL: op = Operator::Equal; break;
+        case PLUSEQUAL: op = Operator::PlusEq; break;
+        case MINUSEQUAL: op = Operator::MinusEq; break;
+        case MULTEQUAL: op = Operator::MultEq; break;
+        case DIVEQUAL: op = Operator::DivEq; break;
+        case LSHIFTEQUAL: op = Operator::LShift; break;
+        case RSHIFTEQUAL: op = Operator::RShift; break;
+        case URSHIFTEQUAL: op = Operator::URShift; break;
+        case BITANDEQUAL: op = Operator::BitAndEq; break;
+        case BITXOREQUAL: op = Operator::BitXOrEq; break;
+        case BITOREQUAL: op = Operator::BitOrEq; break;
+        case MODEQUAL: op = Operator::ModEq; break;
+        case POWEQUAL: op = Operator::PowEq; break;
         default:
             goto end;
         }
@@ -5117,7 +5117,7 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
         semanticFailIfFalse(isSimpleAssignmentTarget(context, expr), "Postfix ++ operator applied to value that is not a reference");
         m_parserState.nonTrivialExpressionCount++;
         m_parserState.nonLHSCount++;
-        expr = context.makePostfixNode(location, expr, OpPlusPlus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
+        expr = context.makePostfixNode(location, expr, Operator::PlusPlus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
         m_parserState.assignmentCount++;
         failIfTrueIfStrict(isEvalOrArguments, "Cannot modify '", m_parserState.lastIdentifier->impl(), "' in strict mode");
         semanticFailIfTrue(hasPrefixUpdateOp, "The ", operatorString(false, lastOperator), " operator requires a reference expression");
@@ -5128,7 +5128,7 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
         semanticFailIfFalse(isSimpleAssignmentTarget(context, expr), "Postfix -- operator applied to value that is not a reference");
         m_parserState.nonTrivialExpressionCount++;
         m_parserState.nonLHSCount++;
-        expr = context.makePostfixNode(location, expr, OpMinusMinus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
+        expr = context.makePostfixNode(location, expr, Operator::MinusMinus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
         m_parserState.assignmentCount++;
         failIfTrueIfStrict(isEvalOrArguments, "'", m_parserState.lastIdentifier->impl(), "' cannot be modified in strict mode");
         semanticFailIfTrue(hasPrefixUpdateOp, "The ", operatorString(false, lastOperator), " operator requires a reference expression");
@@ -5158,13 +5158,13 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
         case PLUSPLUS:
         case AUTOPLUSPLUS:
             ASSERT(isSimpleAssignmentTarget(context, expr));
-            expr = context.makePrefixNode(location, expr, OpPlusPlus, subExprStart, subExprStart + 2, end);
+            expr = context.makePrefixNode(location, expr, Operator::PlusPlus, subExprStart, subExprStart + 2, end);
             m_parserState.assignmentCount++;
             break;
         case MINUSMINUS:
         case AUTOMINUSMINUS:
             ASSERT(isSimpleAssignmentTarget(context, expr));
-            expr = context.makePrefixNode(location, expr, OpMinusMinus, subExprStart, subExprStart + 2, end);
+            expr = context.makePrefixNode(location, expr, Operator::MinusMinus, subExprStart, subExprStart + 2, end);
             m_parserState.assignmentCount++;
             break;
         case TYPEOF: