Remove redundant ids from AST

src/ast.h

Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index c5a924c43d914f10246f7b8513a0cb0417dfae01..fe2a68ee2808de20c9b017eb849db245a369ab49 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -375,15 +375,15 @@ class Expression : public AstNode {
   byte to_boolean_types() const { return to_boolean_types_; }
 
   BailoutId id() const { return id_; }
-  TypeFeedbackId test_id() const { return test_id_; }
+  TypeFeedbackId test_id() const { return TypeFeedbackId(id_.ToInt() + 1); }
 
  protected:
   Expression(Zone* zone, int pos, IdGen* id_gen)
       : AstNode(pos),
         bounds_(Bounds::Unbounded(zone)),
         parenthesization_level_(0),
-        id_(id_gen->GetNextId()),
-        test_id_(id_gen->GetNextId()) {}
+        id_(id_gen->ReserveIdRange(2)) {}  // Reserve id and test id
+
   void set_to_boolean_types(byte types) { to_boolean_types_ = types; }
 
  private:
@@ -392,7 +392,6 @@ class Expression : public AstNode {
   unsigned parenthesization_level_;
 
   const BailoutId id_;
-  const TypeFeedbackId test_id_;
 };
 
 
@@ -421,7 +420,7 @@ class BreakableStatement : public Statement {
   }
 
   BailoutId EntryId() const { return entry_id_; }
-  BailoutId ExitId() const { return exit_id_; }
+  BailoutId ExitId() const { return BailoutId(entry_id_.ToInt() + 1); }
 
  protected:
   BreakableStatement(Zone* zone, ZoneList<const AstRawString*>* labels,
@@ -429,8 +428,7 @@ class BreakableStatement : public Statement {
       : Statement(zone, position),
         labels_(labels),
         breakable_type_(breakable_type),
-        entry_id_(id_gen->GetNextId()),
-        exit_id_(id_gen->GetNextId()) {
+        entry_id_(id_gen->ReserveIdRange(2)) {  // Reserve entry and exit ids
     DCHECK(labels == NULL || labels->length() > 0);
   }
 
@@ -440,7 +438,6 @@ class BreakableStatement : public Statement {
   BreakableType breakable_type_;
   Label break_target_;
   const BailoutId entry_id_;
-  const BailoutId exit_id_;
 };
 
 
@@ -455,7 +452,7 @@ class Block FINAL : public BreakableStatement {
   ZoneList<Statement*>* statements() { return &statements_; }
   bool is_initializer_block() const { return is_initializer_block_; }
 
-  BailoutId DeclsId() const { return decls_id_; }
+  BailoutId DeclsId() const { return BailoutId(EntryId().ToInt() + 2); }

[marja, 2014/09/12 12:51:55]

This looks a bit fragile... now if somebody adds an ID to the super class, or
removes one, they need to update all the magic numbers in the subclasses :(

Can this be made less fragile, e.g, by introducing LastId() functions, and then
this for example would be:

return BailoutId(this->BreakableStatement::LastId() + 1);

Or maybe something better than that? Something to get rid of magic numbers. 1 is
not a magic number :)

[oetuaho-nv, 2014/09/12 13:26:29]

Yep, the thought crossed my mind that there might be a better way to express
this, but I decided to start by making a minimal change. The DCHECKs will of
course guard against serious breakage.

Something like the LastId() suggestion would be better, that would make the
classes more self-contained. And I suppose the compiler would be able to inline
it and optimize a+1+1 to be a+2. Or maybe it could be NextId() instead of
LastId(). I'll work on an improved version of the change.

 
   virtual bool IsJump() const OVERRIDE {
     return !statements_.is_empty() && statements_.last()->IsJump()
@@ -471,13 +468,17 @@ class Block FINAL : public BreakableStatement {
       : BreakableStatement(zone, labels, TARGET_FOR_NAMED_ONLY, pos, id_gen),
         statements_(capacity, zone),
         is_initializer_block_(is_initializer_block),
-        decls_id_(id_gen->GetNextId()),
-        scope_(NULL) {}
+        scope_(NULL) {
+#ifdef DEBUG
+    int decls_id =
+#endif
+    id_gen->GetNextId();  // Reserve decls id
+    DCHECK(decls_id == EntryId().ToInt() + 2);
+  }
 
  private:
   ZoneList<Statement*> statements_;
   bool is_initializer_block_;
-  const BailoutId decls_id_;
   Scope* scope_;
 };
 
@@ -729,7 +730,7 @@ class IterationStatement : public BreakableStatement {
 
   Statement* body() const { return body_; }
 
-  BailoutId OsrEntryId() const { return osr_entry_id_; }
+  BailoutId OsrEntryId() const { return BailoutId(EntryId().ToInt() + 2); }
   virtual BailoutId ContinueId() const = 0;
   virtual BailoutId StackCheckId() const = 0;
 
@@ -740,8 +741,13 @@ class IterationStatement : public BreakableStatement {
   IterationStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
                      IdGen* id_gen)
       : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos, id_gen),
-        body_(NULL),
-        osr_entry_id_(id_gen->GetNextId()) {}
+        body_(NULL) {
+#ifdef DEBUG
+    int id =
+#endif
+    id_gen->GetNextId();  // Reserve osr entry id
+    DCHECK(id == EntryId().ToInt() + 2);
+  }
 
   void Initialize(Statement* body) {
     body_ = body;
@@ -750,8 +756,6 @@ class IterationStatement : public BreakableStatement {
  private:
   Statement* body_;
   Label continue_target_;
-
-  const BailoutId osr_entry_id_;
 };
 
 
@@ -766,23 +770,29 @@ class DoWhileStatement FINAL : public IterationStatement {
 
   Expression* cond() const { return cond_; }
 
-  virtual BailoutId ContinueId() const OVERRIDE { return continue_id_; }
-  virtual BailoutId StackCheckId() const OVERRIDE { return back_edge_id_; }
-  BailoutId BackEdgeId() const { return back_edge_id_; }
+  virtual BailoutId ContinueId() const OVERRIDE {
+    return BailoutId(EntryId().ToInt() + 3);
+  }
+  virtual BailoutId StackCheckId() const OVERRIDE {
+    return BailoutId(EntryId().ToInt() + 4);
+  }
+  BailoutId BackEdgeId() const { return BailoutId(EntryId().ToInt() + 4); }
 
  protected:
   DoWhileStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
                    IdGen* id_gen)
       : IterationStatement(zone, labels, pos, id_gen),
-        cond_(NULL),
-        continue_id_(id_gen->GetNextId()),
-        back_edge_id_(id_gen->GetNextId()) {}
+        cond_(NULL) {
+    // reserve continue and back edge ids
+#ifdef DEBUG
+    int continue_id =
+#endif
+    id_gen->ReserveIdRange(2);
+    DCHECK(continue_id == EntryId().ToInt() + 3);
+  }
 
  private:
   Expression* cond_;
-
-  const BailoutId continue_id_;
-  const BailoutId back_edge_id_;
 };
 
 
@@ -804,24 +814,29 @@ class WhileStatement FINAL : public IterationStatement {
   }
 
   virtual BailoutId ContinueId() const OVERRIDE { return EntryId(); }
-  virtual BailoutId StackCheckId() const OVERRIDE { return body_id_; }
-  BailoutId BodyId() const { return body_id_; }
+  virtual BailoutId StackCheckId() const OVERRIDE {
+    return BailoutId(EntryId().ToInt() + 3);
+  }
+  BailoutId BodyId() const { return BailoutId(EntryId().ToInt() + 3); }
 
  protected:
   WhileStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
                  IdGen* id_gen)
       : IterationStatement(zone, labels, pos, id_gen),
         cond_(NULL),
-        may_have_function_literal_(true),
-        body_id_(id_gen->GetNextId()) {}
+        may_have_function_literal_(true) {
+#ifdef DEBUG
+    int body_id =
+#endif
+    id_gen->GetNextId();  // Reserve body id
+    DCHECK(body_id == EntryId().ToInt() + 3);
+  }
 
  private:
   Expression* cond_;
 
   // True if there is a function literal subexpression in the condition.
   bool may_have_function_literal_;
-
-  const BailoutId body_id_;
 };
 
 
@@ -850,9 +865,13 @@ class ForStatement FINAL : public IterationStatement {
     may_have_function_literal_ = value;
   }
 
-  virtual BailoutId ContinueId() const OVERRIDE { return continue_id_; }
-  virtual BailoutId StackCheckId() const OVERRIDE { return body_id_; }
-  BailoutId BodyId() const { return body_id_; }
+  virtual BailoutId ContinueId() const OVERRIDE {
+    return BailoutId(EntryId().ToInt() + 3);
+  }
+  virtual BailoutId StackCheckId() const OVERRIDE {
+    return BailoutId(EntryId().ToInt() + 4);
+  }
+  BailoutId BodyId() const { return BailoutId(EntryId().ToInt() + 4); }
 
   bool is_fast_smi_loop() { return loop_variable_ != NULL; }
   Variable* loop_variable() { return loop_variable_; }
@@ -866,9 +885,13 @@ class ForStatement FINAL : public IterationStatement {
         cond_(NULL),
         next_(NULL),
         may_have_function_literal_(true),
-        loop_variable_(NULL),
-        continue_id_(id_gen->GetNextId()),
-        body_id_(id_gen->GetNextId()) {}
+        loop_variable_(NULL) {
+#ifdef DEBUG
+    int continue_id =
+#endif
+    id_gen->ReserveIdRange(2);  // Reserve continue and body ids
+    DCHECK(continue_id == EntryId().ToInt() + 3);
+  }
 
  private:
   Statement* init_;
@@ -878,9 +901,6 @@ class ForStatement FINAL : public IterationStatement {
   // True if there is a function literal subexpression in the condition.
   bool may_have_function_literal_;
   Variable* loop_variable_;
-
-  const BailoutId continue_id_;
-  const BailoutId body_id_;
 };
 
 
@@ -935,24 +955,26 @@ class ForInStatement FINAL : public ForEachStatement,
   ForInType for_in_type() const { return for_in_type_; }
   void set_for_in_type(ForInType type) { for_in_type_ = type; }
 
-  BailoutId BodyId() const { return body_id_; }
-  BailoutId PrepareId() const { return prepare_id_; }
+  BailoutId BodyId() const { return BailoutId(EntryId().ToInt() + 3); }
+  BailoutId PrepareId() const { return BailoutId(EntryId().ToInt() + 4); }
   virtual BailoutId ContinueId() const OVERRIDE { return EntryId(); }
-  virtual BailoutId StackCheckId() const OVERRIDE { return body_id_; }
+  virtual BailoutId StackCheckId() const OVERRIDE { return BodyId(); }
 
  protected:
   ForInStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
                  IdGen* id_gen)
       : ForEachStatement(zone, labels, pos, id_gen),
         for_in_type_(SLOW_FOR_IN),
-        for_in_feedback_slot_(kInvalidFeedbackSlot),
-        body_id_(id_gen->GetNextId()),
-        prepare_id_(id_gen->GetNextId()) {}
+        for_in_feedback_slot_(kInvalidFeedbackSlot) {
+#ifdef DEBUG
+    int body_id =
+#endif
+    id_gen->ReserveIdRange(2);  // reserve body and prepare ids
+    DCHECK(body_id == EntryId().ToInt() + 3);
+  }
 
   ForInType for_in_type_;
   int for_in_feedback_slot_;
-  const BailoutId body_id_;
-  const BailoutId prepare_id_;
 };
 
 
@@ -1001,7 +1023,7 @@ class ForOfStatement FINAL : public ForEachStatement {
   virtual BailoutId ContinueId() const OVERRIDE { return EntryId(); }
   virtual BailoutId StackCheckId() const OVERRIDE { return BackEdgeId(); }
 
-  BailoutId BackEdgeId() const { return back_edge_id_; }
+  BailoutId BackEdgeId() const { return BailoutId(EntryId().ToInt() + 3); }
 
  protected:
   ForOfStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
@@ -1010,14 +1032,18 @@ class ForOfStatement FINAL : public ForEachStatement {
         assign_iterator_(NULL),
         next_result_(NULL),
         result_done_(NULL),
-        assign_each_(NULL),
-        back_edge_id_(id_gen->GetNextId()) {}
+        assign_each_(NULL) {
+#ifdef DEBUG
+    int back_edge_id =
+#endif
+    id_gen->GetNextId();  // reserve back edge id
+    DCHECK(back_edge_id == EntryId().ToInt() + 3);
+  }
 
   Expression* assign_iterator_;
   Expression* next_result_;
   Expression* result_done_;
   Expression* assign_each_;
-  const BailoutId back_edge_id_;
 };
 
 
@@ -1128,10 +1154,10 @@ class CaseClause FINAL : public Expression {
   Label* body_target() { return &body_target_; }
   ZoneList<Statement*>* statements() const { return statements_; }
 
-  BailoutId EntryId() const { return entry_id_; }
+  BailoutId EntryId() const { return BailoutId(id().ToInt() + 3); }
 
   // Type feedback information.
-  TypeFeedbackId CompareId() { return compare_id_; }
+  TypeFeedbackId CompareId() { return TypeFeedbackId(id().ToInt() + 2); }
   Type* compare_type() { return compare_type_; }
   void set_compare_type(Type* type) { compare_type_ = type; }
 
@@ -1143,9 +1169,6 @@ class CaseClause FINAL : public Expression {
   Label body_target_;
   ZoneList<Statement*>* statements_;
   Type* compare_type_;
-
-  const TypeFeedbackId compare_id_;
-  const BailoutId entry_id_;
 };
 
 
@@ -1196,8 +1219,8 @@ class IfStatement FINAL : public Statement {
   }
 
   BailoutId IfId() const { return if_id_; }
-  BailoutId ThenId() const { return then_id_; }
-  BailoutId ElseId() const { return else_id_; }
+  BailoutId ThenId() const { return BailoutId(if_id_.ToInt() + 1); }
+  BailoutId ElseId() const { return BailoutId(if_id_.ToInt() + 2); }
 
  protected:
   IfStatement(Zone* zone, Expression* condition, Statement* then_statement,
@@ -1206,17 +1229,13 @@ class IfStatement FINAL : public Statement {
         condition_(condition),
         then_statement_(then_statement),
         else_statement_(else_statement),
-        if_id_(id_gen->GetNextId()),
-        then_id_(id_gen->GetNextId()),
-        else_id_(id_gen->GetNextId()) {}
+        if_id_(id_gen->ReserveIdRange(3)) {}  // Reserve if, then, else ids
 
  private:
   Expression* condition_;
   Statement* then_statement_;
   Statement* else_statement_;
   const BailoutId if_id_;
-  const BailoutId then_id_;
-  const BailoutId else_id_;
 };
 
 
@@ -1791,7 +1810,7 @@ class Call FINAL : public Expression, public FeedbackSlotInterface {
   }
   bool ComputeGlobalTarget(Handle<GlobalObject> global, LookupIterator* it);
 
-  BailoutId ReturnId() const { return return_id_; }
+  BailoutId ReturnId() const { return BailoutId(id().ToInt() + 2); }
 
   enum CallType {
     POSSIBLY_EVAL_CALL,
@@ -1816,8 +1835,12 @@ class Call FINAL : public Expression, public FeedbackSlotInterface {
       : Expression(zone, pos, id_gen),
         expression_(expression),
         arguments_(arguments),
-        call_feedback_slot_(kInvalidFeedbackSlot),
-        return_id_(id_gen->GetNextId()) {
+        call_feedback_slot_(kInvalidFeedbackSlot) {
+#ifdef DEBUG
+    int return_id =
+#endif
+    id_gen->GetNextId();  // Reserve return id
+    DCHECK(return_id == id().ToInt() + 2);
     if (expression->IsProperty()) {
       expression->AsProperty()->mark_for_call();
     }
@@ -1831,8 +1854,6 @@ class Call FINAL : public Expression, public FeedbackSlotInterface {
   Handle<Cell> cell_;
   Handle<AllocationSite> allocation_site_;
   int call_feedback_slot_;
-
-  const BailoutId return_id_;
 };
 
 
@@ -1952,8 +1973,10 @@ class UnaryOperation FINAL : public Expression {
   Token::Value op() const { return op_; }
   Expression* expression() const { return expression_; }
 
-  BailoutId MaterializeTrueId() { return materialize_true_id_; }
-  BailoutId MaterializeFalseId() { return materialize_false_id_; }
+  // For unary not (Token::NOT), the AST ids where true and false will
+  // actually be materialized, respectively.
+  BailoutId MaterializeTrueId() { return BailoutId(id().ToInt() + 2); }
+  BailoutId MaterializeFalseId() { return BailoutId(id().ToInt() + 3); }
 
   virtual void RecordToBooleanTypeFeedback(
       TypeFeedbackOracle* oracle) OVERRIDE;
@@ -1963,20 +1986,19 @@ class UnaryOperation FINAL : public Expression {
                  IdGen* id_gen)
       : Expression(zone, pos, id_gen),
         op_(op),
-        expression_(expression),
-        materialize_true_id_(id_gen->GetNextId()),
-        materialize_false_id_(id_gen->GetNextId()) {
+        expression_(expression) {
+    // Reserve materialize true and false ids
+#ifdef DEBUG
+    int materialize_true_id =
+#endif
+    id_gen->ReserveIdRange(2);
+    DCHECK(materialize_true_id == id().ToInt() + 2);
     DCHECK(Token::IsUnaryOp(op));
   }
 
  private:
   Token::Value op_;
   Expression* expression_;
-
-  // For unary not (Token::NOT), the AST ids where true and false will
-  // actually be materialized, respectively.
-  const BailoutId materialize_true_id_;
-  const BailoutId materialize_false_id_;
 };
 
 
@@ -1994,7 +2016,9 @@ class BinaryOperation FINAL : public Expression {
     allocation_site_ = allocation_site;
   }
 
-  BailoutId RightId() const { return right_id_; }
+  // The short-circuit logical operations need an AST ID for their
+  // right-hand subexpression.
+  BailoutId RightId() const { return BailoutId(id().ToInt() + 2); }
 
   TypeFeedbackId BinaryOperationFeedbackId() const { return reuse(id()); }
   Maybe<int> fixed_right_arg() const { return fixed_right_arg_; }
@@ -2009,8 +2033,12 @@ class BinaryOperation FINAL : public Expression {
       : Expression(zone, pos, id_gen),
         op_(op),
         left_(left),
-        right_(right),
-        right_id_(id_gen->GetNextId()) {
+        right_(right) {
+#ifdef DEBUG
+    int right_id =
+#endif
+    id_gen->GetNextId();  // Reserve right id
+    DCHECK(right_id == id().ToInt() + 2);
     DCHECK(Token::IsBinaryOp(op));
   }
 
@@ -2023,10 +2051,6 @@ class BinaryOperation FINAL : public Expression {
   // TODO(rossberg): the fixed arg should probably be represented as a Constant
   // type for the RHS.
   Maybe<int> fixed_right_arg_;
-
-  // The short-circuit logical operations need an AST ID for their
-  // right-hand subexpression.
-  const BailoutId right_id_;
 };
 
 
@@ -2057,9 +2081,11 @@ class CountOperation FINAL : public Expression {
   void set_store_mode(KeyedAccessStoreMode mode) { store_mode_ = mode; }
   void set_type(Type* type) { type_ = type; }
 
-  BailoutId AssignmentId() const { return assignment_id_; }
+  BailoutId AssignmentId() const { return BailoutId(id().ToInt() + 2); }
 
-  TypeFeedbackId CountBinOpFeedbackId() const { return count_id_; }
+  TypeFeedbackId CountBinOpFeedbackId() const {
+    return TypeFeedbackId(id().ToInt() + 3);
+  }
   TypeFeedbackId CountStoreFeedbackId() const { return reuse(id()); }
 
  protected:
@@ -2069,9 +2095,14 @@ class CountOperation FINAL : public Expression {
         op_(op),
         is_prefix_(is_prefix),
         store_mode_(STANDARD_STORE),
-        expression_(expr),
-        assignment_id_(id_gen->GetNextId()),
-        count_id_(id_gen->GetNextId()) {}
+        expression_(expr) {
+    // Reserve assignment and count ids
+#ifdef DEBUG
+    int assignment_id =
+#endif
+    id_gen->ReserveIdRange(2);
+    DCHECK(assignment_id == id().ToInt() + 2);
+  }
 
  private:
   Token::Value op_;
@@ -2081,8 +2112,6 @@ class CountOperation FINAL : public Expression {
   Type* type_;
 
   Expression* expression_;
-  const BailoutId assignment_id_;
-  const TypeFeedbackId count_id_;
   SmallMapList receiver_types_;
 };
 
@@ -2133,8 +2162,8 @@ class Conditional FINAL : public Expression {
   Expression* then_expression() const { return then_expression_; }
   Expression* else_expression() const { return else_expression_; }
 
-  BailoutId ThenId() const { return then_id_; }
-  BailoutId ElseId() const { return else_id_; }
+  BailoutId ThenId() const { return BailoutId(id().ToInt() + 2); }
+  BailoutId ElseId() const { return BailoutId(id().ToInt() + 3); }
 
  protected:
   Conditional(Zone* zone, Expression* condition, Expression* then_expression,
@@ -2142,16 +2171,18 @@ class Conditional FINAL : public Expression {
       : Expression(zone, position, id_gen),
         condition_(condition),
         then_expression_(then_expression),
-        else_expression_(else_expression),
-        then_id_(id_gen->GetNextId()),
-        else_id_(id_gen->GetNextId()) {}
+        else_expression_(else_expression) {
+#ifdef DEBUG
+    int then_id =
+#endif
+    id_gen->ReserveIdRange(2);  // Reserve then and else ids
+    DCHECK(then_id == id().ToInt() + 2);
+  }
 
  private:
   Expression* condition_;
   Expression* then_expression_;
   Expression* else_expression_;
-  const BailoutId then_id_;
-  const BailoutId else_id_;
 };
 
 
@@ -2171,7 +2202,7 @@ class Assignment FINAL : public Expression {
   // This check relies on the definition order of token in token.h.
   bool is_compound() const { return op() > Token::ASSIGN; }
 
-  BailoutId AssignmentId() const { return assignment_id_; }
+  BailoutId AssignmentId() const { return BailoutId(id().ToInt() + 2); }
 
   // Type feedback information.
   TypeFeedbackId AssignmentFeedbackId() { return reuse(id()); }
@@ -2209,7 +2240,6 @@ class Assignment FINAL : public Expression {
   Expression* target_;
   Expression* value_;
   BinaryOperation* binary_operation_;
-  const BailoutId assignment_id_;
 
   bool is_uninitialized_ : 1;
   KeyedAccessStoreMode store_mode_ : 5;  // Windows treats as signed,