[v8-dev] Unify handling of position info in AST, part 2 (issue 23597037)

Thu, 12 Sep 2013 07:12:11 -0700 

Reviewers: Yang,

Description:
Unify handling of position info in AST, part 2

* Eliminate Conditional::then/else_position and
WhileStatement::condition_position.

[email protected]
BUG=

Please review this at https://codereview.chromium.org/23597037/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+5, -32 lines):
  M src/ast.h
  M src/full-codegen.cc
  M src/parser.cc


Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index 44332dc7691a35d15e63b5224dc877060e354c74..b3168d4e326ee9f9432c947e978bcb5379a4dc86 100644 
--- a/src/ast.h
+++ b/src/ast.h
@@ -765,12 +765,6 @@ class DoWhileStatement V8_FINAL : public IterationStatement { 

   Expression* cond() const { return cond_; }

-  // TODO(rossberg): get rid of this.
-  // Position where condition expression starts. We need it to make
-  // the loop's condition a breakable location.
-  int condition_position() { return condition_position_; }
-  void set_condition_position(int pos) { condition_position_ = pos; }
-
   virtual BailoutId ContinueId() const V8_OVERRIDE { return continue_id_; }
virtual BailoutId StackCheckId() const V8_OVERRIDE { return back_edge_id_; } 
   BailoutId BackEdgeId() const { return back_edge_id_; }
@@ -779,7 +773,6 @@ class DoWhileStatement V8_FINAL : public IterationStatement { 
   DoWhileStatement(Isolate* isolate, ZoneStringList* labels, int pos)
       : IterationStatement(isolate, labels, pos),
         cond_(NULL),
-        condition_position_(-1),
         continue_id_(GetNextId(isolate)),
         back_edge_id_(GetNextId(isolate)) {
   }
@@ -787,8 +780,6 @@ class DoWhileStatement V8_FINAL : public IterationStatement { 
  private:
   Expression* cond_;

-  int condition_position_;
-
   const BailoutId continue_id_;
   const BailoutId back_edge_id_;
 };
@@ -2061,10 +2052,6 @@ class Conditional V8_FINAL : public Expression {
   Expression* then_expression() const { return then_expression_; }
   Expression* else_expression() const { return else_expression_; }

-  // TODO(rossberg): get rid of this.
- int then_expression_position() const { return then_expression_position_; } - int else_expression_position() const { return else_expression_position_; } 
-
   BailoutId ThenId() const { return then_id_; }
   BailoutId ElseId() const { return else_id_; }

@@ -2073,15 +2060,11 @@ class Conditional V8_FINAL : public Expression {
               Expression* condition,
               Expression* then_expression,
               Expression* else_expression,
-              int then_expression_position,
-              int else_expression_position,
               int position)
       : Expression(isolate, position),
         condition_(condition),
         then_expression_(then_expression),
         else_expression_(else_expression),
-        then_expression_position_(then_expression_position),
-        else_expression_position_(else_expression_position),
         then_id_(GetNextId(isolate)),
         else_id_(GetNextId(isolate)) { }

@@ -2089,8 +2072,6 @@ class Conditional V8_FINAL : public Expression {
   Expression* condition_;
   Expression* then_expression_;
   Expression* else_expression_;
-  int then_expression_position_;
-  int else_expression_position_;
   const BailoutId then_id_;
   const BailoutId else_id_;
 };
@@ -3198,12 +3179,9 @@ class AstNodeFactory V8_FINAL BASE_EMBEDDED {
   Conditional* NewConditional(Expression* condition,
                               Expression* then_expression,
                               Expression* else_expression,
-                              int then_expression_position,
-                              int else_expression_position,
                               int position) {
     Conditional* cond = new(zone_) Conditional(
-        isolate_, condition, then_expression, else_expression,
-        then_expression_position, else_expression_position, position);
+        isolate_, condition, then_expression, else_expression, position);
     VISIT_AND_RETURN(Conditional, cond)
   }

Index: src/full-codegen.cc
diff --git a/src/full-codegen.cc b/src/full-codegen.cc
index 58ad3dc814914efe555a8e9eed1924cd4c991555..7f82a01769cef962e1d64b63a2a288a5f56680ef 100644 
--- a/src/full-codegen.cc
+++ b/src/full-codegen.cc
@@ -1293,7 +1293,7 @@ void FullCodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) { 
   // possible to break on the condition.
   __ bind(loop_statement.continue_label());
   PrepareForBailoutForId(stmt->ContinueId(), NO_REGISTERS);
-  SetExpressionPosition(stmt->cond(), stmt->condition_position());
+  SetExpressionPosition(stmt->cond(), stmt->cond()->position());
   VisitForControl(stmt->cond(),
                   &book_keeping,
                   loop_statement.break_label(),
@@ -1523,7 +1523,7 @@ void FullCodeGenerator::VisitConditional(Conditional* expr) { 
   PrepareForBailoutForId(expr->ThenId(), NO_REGISTERS);
   __ bind(&true_case);
   SetExpressionPosition(expr->then_expression(),
-                        expr->then_expression_position());
+                        expr->then_expression()->position());
   if (context()->IsTest()) {
     const TestContext* for_test = TestContext::cast(context());
     VisitForControl(expr->then_expression(),
@@ -1538,7 +1538,7 @@ void FullCodeGenerator::VisitConditional(Conditional* expr) { 
   PrepareForBailoutForId(expr->ElseId(), NO_REGISTERS);
   __ bind(&false_case);
   SetExpressionPosition(expr->else_expression(),
-                        expr->else_expression_position());
+                        expr->else_expression()->position());
   VisitInDuplicateContext(expr->else_expression());
   // If control flow falls through Visit, merge it with true case here.
   if (!context()->IsTest()) {
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 8b467e939e03675fa0d38f09090358813a3f0575..1c5435c0ee8ffc3cb3d9678c1c095a2d33cd7d43 100644 
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2592,8 +2592,6 @@ DoWhileStatement* Parser::ParseDoWhileStatement(ZoneStringList* labels, 
   Expect(Token::WHILE, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);

-  if (loop != NULL) loop->set_condition_position(position());
-
   Expression* cond = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
@@ -3045,13 +3043,10 @@ Expression* Parser::ParseConditionalExpression(bool accept_IN, bool* ok) { 
   // In parsing the first assignment expression in conditional
   // expressions we always accept the 'in' keyword; see ECMA-262,
   // section 11.12, page 58.
-  int left_position = peek_position();
   Expression* left = ParseAssignmentExpression(true, CHECK_OK);
   Expect(Token::COLON, CHECK_OK);
-  int right_position = peek_position();
   Expression* right = ParseAssignmentExpression(accept_IN, CHECK_OK);
-  return factory()->NewConditional(
-      expression, left, right, left_position, right_position, pos);
+  return factory()->NewConditional(expression, left, right, pos);
 }




--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to