Clean up manual bit field usage in PreParserExpression

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index 1c1e1c4520d0e3cad95a6f2cbb3afdec963d02a7..363bcdb4622592a8e202fed1a8c50557abfd51df 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -608,30 +608,27 @@ class PreParserIdentifier {
 };
 
 
-// Bits 0 and 1 are used to identify the type of expression:
-// If bit 0 is set, it's an identifier.
-// if bit 1 is set, it's a string literal.
-// If neither is set, it's no particular type, and both set isn't
-// use yet.
 class PreParserExpression {
  public:
   static PreParserExpression Default() {
-    return PreParserExpression(kUnknownExpression);
+    return PreParserExpression(kExpression);
   }
 
   static PreParserExpression FromIdentifier(PreParserIdentifier id) {
-    return PreParserExpression(kTypeIdentifier |
-                               (id.type_ << kIdentifierShift));
+    PreParserExpression expr(kIdentifierExpression);
+    expr.detail_.identifier_type = id.type_;
+    return expr;
   }
 
   static PreParserExpression BinaryOperation(PreParserExpression left,
                                              Token::Value op,
                                              PreParserExpression right) {
-    int code = ((op == Token::COMMA) && !left.is_parenthesized() &&
-                !right.is_parenthesized())
-                   ? left.ArrowParamListBit() & right.ArrowParamListBit()
-                   : 0;
-    return PreParserExpression(kTypeBinaryOperation | code);
+    PreParserExpression expr(kBinaryOperationExpression);
+    expr.detail_.valid_arrow_param_list =
+        op == Token::COMMA && !left.is_parenthesized() &&
+        !right.is_parenthesized() && left.IsValidArrowParams() &&
+        right.IsValidArrowParams();
+    return expr;
   }
 
   static PreParserExpression EmptyArrowParamList() {
@@ -641,65 +638,79 @@ class PreParserExpression {
   }
 
   static PreParserExpression StringLiteral() {
-    return PreParserExpression(kUnknownStringLiteral);
+    return PreParserExpression(kStringLiteralExpression);
   }
 
   static PreParserExpression UseStrictStringLiteral() {
-    return PreParserExpression(kUseStrictString);
+    PreParserExpression expr(kStringLiteralExpression);
+    expr.detail_.string_literal_is_use_strict = true;
+    return expr;
   }
 
   static PreParserExpression This() {
-    return PreParserExpression(kThisExpression);
+    PreParserExpression expr(kExpression);
+    expr.detail_.expression_type = kThisExpression;
+    return expr;
   }
 
   static PreParserExpression ThisProperty() {
-    return PreParserExpression(kThisPropertyExpression);
+    PreParserExpression expr(kExpression);
+    expr.detail_.expression_type = kThisPropertyExpression;
+    return expr;
   }
 
   static PreParserExpression Property() {
-    return PreParserExpression(kPropertyExpression);
+    PreParserExpression expr(kExpression);
+    expr.detail_.expression_type = kPropertyExpression;
+    return expr;
   }
 
   static PreParserExpression Call() {
-    return PreParserExpression(kCallExpression);
+    PreParserExpression expr(kExpression);
+    expr.detail_.expression_type = kCallExpression;
+    return expr;
   }
 
-  bool IsIdentifier() const { return (code_ & kTypeMask) == kTypeIdentifier; }
+  bool IsIdentifier() const { return type_ == kIdentifierExpression; }
 
   PreParserIdentifier AsIdentifier() const {
     ASSERT(IsIdentifier());
-    return PreParserIdentifier(
-        static_cast<PreParserIdentifier::Type>(code_ >> kIdentifierShift));
+    return PreParserIdentifier(detail_.identifier_type);
   }
 
-  bool IsStringLiteral() const {
-    return (code_ & kTypeMask) == kTypeStringLiteral;
-  }
+  bool IsStringLiteral() const { return type_ == kStringLiteralExpression; }
 
   bool IsUseStrictLiteral() const {
-    return (code_ & kUseStrictString) == kUseStrictString;
+    return type_ == kStringLiteralExpression &&
+           detail_.string_literal_is_use_strict;
   }
 
-  bool IsThis() const { return (code_ & kThisExpression) == kThisExpression; }
+  bool IsThis() const {
+    return type_ == kExpression && detail_.expression_type == kThisExpression;
+  }
 
   bool IsThisProperty() const {
-    return (code_ & kThisPropertyExpression) == kThisPropertyExpression;
+    return type_ == kExpression &&
+           detail_.expression_type == kThisPropertyExpression;
   }
 
   bool IsProperty() const {
-    return (code_ & kPropertyExpression) == kPropertyExpression ||
-           (code_ & kThisPropertyExpression) == kThisPropertyExpression;
+    return type_ == kExpression &&
+           (detail_.expression_type == kPropertyExpression ||
+            detail_.expression_type == kThisPropertyExpression);
   }
 
-  bool IsCall() const { return (code_ & kCallExpression) == kCallExpression; }
+  bool IsCall() const {
+    return type_ == kExpression && detail_.expression_type == kCallExpression;
+  }
 
   bool IsValidReferenceExpression() const {
     return IsIdentifier() || IsProperty();
   }
 
   bool IsValidArrowParamList() const {
-    return (ArrowParamListBit() & kBinaryOperationArrowParamList) != 0 &&
-           (code_ & kMultiParenthesizedExpression) == 0;
+    return IsValidArrowParams() &&
+           parenthesization_ != kMultiParenthesizedExpression;
   }
 
   // At the moment PreParser doesn't track these expression types.
@@ -708,17 +719,15 @@ class PreParserExpression {
 
   PreParserExpression AsFunctionLiteral() { return *this; }
 
-  bool IsBinaryOperation() const {
-    return (code_ & kTypeMask) == kTypeBinaryOperation;
-  }
+  bool IsBinaryOperation() const { return type_ == kBinaryOperationExpression; }
 
   bool is_parenthesized() const {
-    return (code_ & kParenthesizedExpression) != 0;
+    return parenthesization_ != kNotParenthesized;
   }
 
   void increase_parenthesization_level() {
-    code_ |= is_parenthesized() ? kMultiParenthesizedExpression
-                                : kParenthesizedExpression;
+    parenthesization_ = is_parenthesized() ? kMultiParenthesizedExpression
+                                           : kParenthesizedExpression;
   }
 
   // Dummy implementation for making expression->somefunc() work in both Parser
@@ -734,65 +743,58 @@ class PreParserExpression {
   void set_ast_properties(int* ast_properties) {}
   void set_dont_optimize_reason(BailoutReason dont_optimize_reason) {}
 
-  bool operator==(const PreParserExpression& other) const {
-    return code_ == other.code_;
-  }
-  bool operator!=(const PreParserExpression& other) const {
-    return code_ != other.code_;
-  }
-
  private:
-  // Least significant 2 bits are used as expression type. The third least
-  // significant bit tracks whether an expression is parenthesized. If the
-  // expression is an identifier or a string literal, the other bits
-  // describe the type/ (see PreParserIdentifier::Type and string literal
-  // constants below). For binary operations, the other bits are flags
-  // which further describe the contents of the expression.
-  enum {
-    kUnknownExpression = 0,
-    kTypeMask = 1 | 2,
-    kParenthesizedExpression = (1 << 2),
-    kMultiParenthesizedExpression = (1 << 3),
-
-    // Identifiers
-    kTypeIdentifier = 1,  // Used to detect labels.
-    kIdentifierShift = 5,
-    kTypeStringLiteral = 2,  // Used to detect directive prologue.
-    kUnknownStringLiteral = kTypeStringLiteral,
-    kUseStrictString = kTypeStringLiteral | 32,
-    kStringLiteralMask = kUseStrictString,
+  enum Type {
+    kExpression,
+    kIdentifierExpression,
+    kStringLiteralExpression,
+    kBinaryOperationExpression
+  };
 
-    // Binary operations. Those are needed to detect certain keywords and
-    // duplicated identifier in parameter lists for arrow functions, because
-    // they are initially parsed as comma-separated expressions.
-    kTypeBinaryOperation = 3,
-    kBinaryOperationArrowParamList = (1 << 4),
+  enum Parenthesization {
+    kNotParenthesized,
+    kParenthesizedExpression,
+    kMultiParenthesizedExpression
+  };
 
-    // Below here applies if neither identifier nor string literal. Reserve the
-    // 2 least significant bits for flags.
-    kThisExpression = (1 << 4),
-    kThisPropertyExpression = (2 << 4),
-    kPropertyExpression = (3 << 4),
-    kCallExpression = (4 << 4)
+  enum ExpressionType {
+    kThisExpression,
+    kThisPropertyExpression,
+    kPropertyExpression,
+    kCallExpression
   };
 
-  explicit PreParserExpression(int expression_code) : code_(expression_code) {}
+  explicit PreParserExpression(Type type) : type_(type) {}
 
-  V8_INLINE int ArrowParamListBit() const {
-    if (IsBinaryOperation()) return code_ & kBinaryOperationArrowParamList;
+  V8_INLINE bool IsValidArrowParams() const {
+    if (IsBinaryOperation()) return detail_.valid_arrow_param_list;
     if (IsIdentifier()) {
       const PreParserIdentifier ident = AsIdentifier();
       // A valid identifier can be an arrow function parameter list
       // except for eval, arguments, yield, and reserved keywords.
       if (ident.IsEval() || ident.IsArguments() || ident.IsYield() ||
           ident.IsFutureStrictReserved())
-        return 0;
-      return kBinaryOperationArrowParamList;
+        return false;
+      return true;
     }
-    return 0;
+    return false;
   }
 
-  int code_;
+  Type type_ : 2;

[Sven Panne, 2014/08/04 06:55:57]

This actually blows up memory usage instead of reducing it and has several
portability issues. Please use the BitField class from utils.h, like the rest of
v8.

+  Parenthesization parenthesization_ : 2;
+  union {
+    // Used for kExpression
+    ExpressionType expression_type : 2;
+
+    // Used for kStringLiteralExpression
+    bool string_literal_is_use_strict : 1;
+
+    // Used for kBinaryOperationExpression
+    bool valid_arrow_param_list : 1;
+
+    // Used for kIdentifierExpression
+    PreParserIdentifier::Type identifier_type : 10;
+  } detail_;
 };