Revert of Clean up rewriter. (patchset #3 id:40001 of https://codereview.chromium.org/1362333002/ )

Reason for revert:
[Sheriff] Seems to break layout tests:
http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/2073

https://storage.googleapis.com/chromium-layout-test-archives/V8-Blink_Linux_64/2073/layout-test-results/results.html

Please request rebaseline upstream first.

Original issue's description:
> Clean up rewriter.
>
> The main changes are:
> - Fix treatment of loops, which was incorrect and sometimes resulted in
>   the wrong completion value.
> - Get rid of unnecessary variables.
>
> This is in preparation of implementing ES6 completion semantics.
>
> R=rossberg
> BUG=
>
> Committed: https://crrev.com/b466dc14791844b7ae6d1e9ebd00a778965c206d
> Cr-Commit-Position: refs/heads/master@{#30981}

TBR=rossberg@chromium.org,neis@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/1379513002

Cr-Commit-Position: refs/heads/master@{#30992}

src/rewriter.cc

@@ -16,7 +16,9 @@ class Processor: public AstVisitor {
   Processor(Isolate* isolate, Variable* result,
             AstValueFactory* ast_value_factory)
       : result_(result),
+        result_assigned_(false),
         is_set_(false),
+        in_try_(false),
         factory_(ast_value_factory) {
     InitializeAstVisitor(isolate, ast_value_factory->zone());
   }
@@ -24,21 +26,30 @@ class Processor: public AstVisitor {
   virtual ~Processor() { }
 
   void Process(ZoneList<Statement*>* statements);
+  bool result_assigned() const { return result_assigned_; }
 
   AstNodeFactory* factory() { return &factory_; }
 
  private:
   Variable* result_;
 
+  // We are not tracking result usage via the result_'s use
+  // counts (we leave the accurate computation to the
+  // usage analyzer). Instead we simple remember if
+  // there was ever an assignment to result_.
+  bool result_assigned_;
+
   // To avoid storing to .result all the time, we eliminate some of
   // the stores by keeping track of whether or not we're sure .result
   // will be overwritten anyway. This is a bit more tricky than what I
-  // was hoping for.
+  // was hoping for
   bool is_set_;
+  bool in_try_;
 
   AstNodeFactory factory_;
 
   Expression* SetResult(Expression* value) {
+    result_assigned_ = true;
     VariableProxy* result_proxy = factory()->NewVariableProxy(result_);
     return factory()->NewAssignment(
         Token::ASSIGN, result_proxy, value, RelocInfo::kNoPosition);
@@ -77,30 +88,29 @@ void Processor::VisitBlock(Block* node) {
 
 void Processor::VisitExpressionStatement(ExpressionStatement* node) {
   // Rewrite : <x>; -> .result = <x>;
-  if (!is_set_) {
+  if (!is_set_ && !node->expression()->IsThrow()) {
     node->set_expression(SetResult(node->expression()));
-    is_set_ = true;
+    if (!in_try_) is_set_ = true;
   }
 }
 
 
 void Processor::VisitIfStatement(IfStatement* node) {
-  // Rewrite both branches.
-  bool set_after = is_set_;
-  Visit(node->then_statement());
-  bool set_in_then = is_set_;
-  is_set_ = set_after;
+  // Rewrite both then and else parts (reversed).
+  bool save = is_set_;
   Visit(node->else_statement());
-  is_set_ = is_set_ && set_in_then;
+  bool set_after_then = is_set_;
+  is_set_ = save;
+  Visit(node->then_statement());
+  is_set_ = is_set_ && set_after_then;
 }
 
 
 void Processor::VisitIterationStatement(IterationStatement* node) {
   // Rewrite the body.
-  bool set_after = is_set_;
-  is_set_ = false;  // We are in a loop, so we can't rely on [set_after].
+  bool set_after_loop = is_set_;
   Visit(node->body());
-  is_set_ = is_set_ && set_after;
+  is_set_ = is_set_ && set_after_loop;
 }
 
 
@@ -130,32 +140,36 @@ void Processor::VisitForOfStatement(ForOfStatement* node) {
 
 
 void Processor::VisitTryCatchStatement(TryCatchStatement* node) {
-  // Rewrite both try and catch block.
-  bool set_after = is_set_;
-  Visit(node->try_block());
-  bool set_in_try = is_set_;
-  is_set_ = set_after;
+  // Rewrite both try and catch blocks (reversed order).
+  bool set_after_catch = is_set_;
   Visit(node->catch_block());
-  is_set_ = is_set_ && set_in_try;
+  is_set_ = is_set_ && set_after_catch;
+  bool save = in_try_;
+  in_try_ = true;
+  Visit(node->try_block());
+  in_try_ = save;
 }
 
 
 void Processor::VisitTryFinallyStatement(TryFinallyStatement* node) {
-  // Rewrite both try and finally block (in reverse order).
+  // Rewrite both try and finally block (reversed order).
   Visit(node->finally_block());
+  bool save = in_try_;
+  in_try_ = true;
   Visit(node->try_block());
+  in_try_ = save;
 }
 
 
 void Processor::VisitSwitchStatement(SwitchStatement* node) {
-  // Rewrite statements in all case clauses (in reverse order).
+  // Rewrite statements in all case clauses in reversed order.
   ZoneList<CaseClause*>* clauses = node->cases();
-  bool set_after = is_set_;
+  bool set_after_switch = is_set_;
   for (int i = clauses->length() - 1; i >= 0; --i) {
     CaseClause* clause = clauses->at(i);
     Process(clause->statements());
   }
-  is_set_ = is_set_ && set_after;
+  is_set_ = is_set_ && set_after_switch;
 }
 
 
@@ -170,7 +184,9 @@ void Processor::VisitBreakStatement(BreakStatement* node) {
 
 
 void Processor::VisitWithStatement(WithStatement* node) {
+  bool set_after_body = is_set_;
   Visit(node->statement());
+  is_set_ = is_set_ && set_after_body;
 }
 
 
@@ -180,28 +196,23 @@ void Processor::VisitSloppyBlockFunctionStatement(
 }
 
 
-void Processor::VisitReturnStatement(ReturnStatement* node) { is_set_ = true; }
-
-
 // Do nothing:
+void Processor::VisitVariableDeclaration(VariableDeclaration* node) {}
+void Processor::VisitFunctionDeclaration(FunctionDeclaration* node) {}
+void Processor::VisitImportDeclaration(ImportDeclaration* node) {}
+void Processor::VisitExportDeclaration(ExportDeclaration* node) {}
 void Processor::VisitEmptyStatement(EmptyStatement* node) {}
+void Processor::VisitReturnStatement(ReturnStatement* node) {}
 void Processor::VisitDebuggerStatement(DebuggerStatement* node) {}
 
 
-// Expressions are never visited.
+// Expressions are never visited yet.
 #define DEF_VISIT(type)                                         \
   void Processor::Visit##type(type* expr) { UNREACHABLE(); }
 EXPRESSION_NODE_LIST(DEF_VISIT)
 #undef DEF_VISIT
 
 
-// Declarations are never visited.
-#define DEF_VISIT(type) \
-  void Processor::Visit##type(type* expr) { UNREACHABLE(); }
-DECLARATION_NODE_LIST(DEF_VISIT)
-#undef DEF_VISIT
-
-
 // Assumes code has been parsed.  Mutates the AST, so the AST should not
 // continue to be used in the case of failure.
 bool Rewriter::Rewrite(ParseInfo* info) {
@@ -221,19 +232,21 @@ bool Rewriter::Rewrite(ParseInfo* info) {
     processor.Process(body);
     if (processor.HasStackOverflow()) return false;
 
-    DCHECK(function->end_position() != RelocInfo::kNoPosition);
-    // Set the position of the assignment statement one character past the
-    // source code, such that it definitely is not in the source code range
-    // of an immediate inner scope. For example in
-    //   eval('with ({x:1}) x = 1');
-    // the end position of the function generated for executing the eval code
-    // coincides with the end of the with scope which is the position of '1'.
-    int pos = function->end_position();
-    VariableProxy* result_proxy =
-        processor.factory()->NewVariableProxy(result, pos);
-    Statement* result_statement =
-        processor.factory()->NewReturnStatement(result_proxy, pos);
-    body->Add(result_statement, info->zone());
+    if (processor.result_assigned()) {
+      DCHECK(function->end_position() != RelocInfo::kNoPosition);
+      // Set the position of the assignment statement one character past the
+      // source code, such that it definitely is not in the source code range
+      // of an immediate inner scope. For example in
+      //   eval('with ({x:1}) x = 1');
+      // the end position of the function generated for executing the eval code
+      // coincides with the end of the with scope which is the position of '1'.
+      int pos = function->end_position();
+      VariableProxy* result_proxy =
+          processor.factory()->NewVariableProxy(result, pos);
+      Statement* result_statement =
+          processor.factory()->NewReturnStatement(result_proxy, pos);
+      body->Add(result_statement, info->zone());
+    }
   }
 
   return true;

test/webkit/eval-throw-return-expected.txt

@@ -28,18 +28,18 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS eval("1;") is 1
 PASS eval("1; try { foo = [2,3,throwFunc(), 4]; } catch (e){}") is 1
-PASS eval("1; try { 2; throw \"\"; } catch (e){}") is 1
-PASS eval("1; try { 2; throwFunc(); } catch (e){}") is 1
+PASS eval("1; try { 2; throw \"\"; } catch (e){}") is 2
+PASS eval("1; try { 2; throwFunc(); } catch (e){}") is 2
 PASS eval("1; try { 2; throwFunc(); } catch (e){3;} finally {}") is 3
 PASS eval("1; try { 2; throwFunc(); } catch (e){3;} finally {4;}") is 4
 PASS eval("function blah() { 1; }\n blah();") is undefined
 PASS eval("var x = 1;") is undefined
 PASS eval("if (true) { 1; } else { 2; }") is 1
 PASS eval("if (false) { 1; } else { 2; }") is 2
-PASS eval("try{1; if (true) { 2; throw \"\"; } else { 2; }} catch(e){}") is undefined
+PASS eval("try{1; if (true) { 2; throw \"\"; } else { 2; }} catch(e){}") is 2
 PASS eval("1; var i = 0; do { ++i; 2; } while(i!=1);") is 2
-#PASS eval("try{1; var i = 0; do { ++i; 2; throw \"\"; } while(i!=1);} catch(e){}") is undefined
-PASS eval("1; try{2; throwOnReturn();} catch(e){}") is 1
+PASS eval("try{1; var i = 0; do { ++i; 2; throw \"\"; } while(i!=1);} catch(e){}") is 2
+PASS eval("1; try{2; throwOnReturn();} catch(e){}") is 2
 PASS eval("1; twoFunc();") is undefined
 PASS eval("1; with ( { a: 0 } ) { 2; }") is 2
 PASS successfullyParsed is true

test/webkit/eval-throw-return.js

@@ -38,17 +38,17 @@ function twoFunc() {
 
 shouldBe('eval("1;")', "1");
 shouldBe('eval("1; try { foo = [2,3,throwFunc(), 4]; } catch (e){}")', "1");
-shouldBe('eval("1; try { 2; throw \\"\\"; } catch (e){}")', "1");
-shouldBe('eval("1; try { 2; throwFunc(); } catch (e){}")', "1");
+shouldBe('eval("1; try { 2; throw \\"\\"; } catch (e){}")', "2");
+shouldBe('eval("1; try { 2; throwFunc(); } catch (e){}")', "2");
 shouldBe('eval("1; try { 2; throwFunc(); } catch (e){3;} finally {}")', "3");
 shouldBe('eval("1; try { 2; throwFunc(); } catch (e){3;} finally {4;}")', "4");
 shouldBe('eval("function blah() { 1; }\\n blah();")', "undefined");
 shouldBe('eval("var x = 1;")', "undefined");
 shouldBe('eval("if (true) { 1; } else { 2; }")', "1");
 shouldBe('eval("if (false) { 1; } else { 2; }")', "2");
-shouldBe('eval("try{1; if (true) { 2; throw \\"\\"; } else { 2; }} catch(e){}")', "undefined");
+shouldBe('eval("try{1; if (true) { 2; throw \\"\\"; } else { 2; }} catch(e){}")', "2");
 shouldBe('eval("1; var i = 0; do { ++i; 2; } while(i!=1);")', "2");
-//shouldBe('eval("try{1; var i = 0; do { ++i; 2; throw \\"\\"; } while(i!=1);} catch(e){}")', "undefined");
-shouldBe('eval("1; try{2; throwOnReturn();} catch(e){}")', "1");
+shouldBe('eval("try{1; var i = 0; do { ++i; 2; throw \\"\\"; } while(i!=1);} catch(e){}")', "2");
+shouldBe('eval("1; try{2; throwOnReturn();} catch(e){}")', "2");
 shouldBe('eval("1; twoFunc();")', "undefined");
 shouldBe('eval("1; with ( { a: 0 } ) { 2; }")', "2");