Fix completion of try..finally.

src/rewriter.cc

Index: src/rewriter.cc
diff --git a/src/rewriter.cc b/src/rewriter.cc
index 11af2bd26246d6fb44ee3fa5cd734f87071dff29..854621ed63ade4718a7ccf57f21860c9f2a15a11 100644
--- a/src/rewriter.cc
+++ b/src/rewriter.cc
@@ -13,12 +13,13 @@ namespace internal {
 
 class Processor: public AstVisitor {
  public:
-  Processor(Isolate* isolate, Variable* result,
+  Processor(Isolate* isolate, Scope* scope, Variable* result,
             AstValueFactory* ast_value_factory)
       : result_(result),
         result_assigned_(false),
         replacement_(nullptr),
         is_set_(false),
+        scope_(scope),
         factory_(ast_value_factory) {
     InitializeAstVisitor(isolate, ast_value_factory->zone());
   }
@@ -28,6 +29,7 @@ class Processor: public AstVisitor {
   void Process(ZoneList<Statement*>* statements);
   bool result_assigned() const { return result_assigned_; }
 
+  Scope* scope() { return scope_; }
   AstNodeFactory* factory() { return &factory_; }
 
  private:
@@ -49,8 +51,10 @@ class Processor: public AstVisitor {
   // was hoping for.
   bool is_set_;
 
+  Scope* scope_;
   AstNodeFactory factory_;
 
+  // Returns ".result = value"
   Expression* SetResult(Expression* value) {
     result_assigned_ = true;
     VariableProxy* result_proxy = factory()->NewVariableProxy(result_);
@@ -58,6 +62,9 @@ class Processor: public AstVisitor {
         Token::ASSIGN, result_proxy, value, RelocInfo::kNoPosition);
   }
 
+  // Changes b to ".backup = .result; b; .result = .backup"
+  void BackupAndRestore(Block* b);

[rossberg, 2015/10/01 09:59:24]

Nit: SaveAndRestore would sound more like the typical phrase. But in fact, I'd
just inline this function

[neis, 2015/10/01 10:56:48]

Did that, but I don't think it improved readability.

+
   // Node visitors.
 #define DEF_VISIT(type) virtual void Visit##type(type* node) override;
   AST_NODE_LIST(DEF_VISIT)
@@ -69,6 +76,24 @@ class Processor: public AstVisitor {
 };
 
 
+void Processor::BackupAndRestore(Block* b) {
+  Variable* backup = scope()->NewTemporary(
+      factory()->ast_value_factory()->dot_result_string());
+  Expression* backup_proxy = factory()->NewVariableProxy(backup);
+  Expression* result_proxy = factory()->NewVariableProxy(result_);
+  Expression* assignment1 = factory()->NewAssignment(
+      Token::ASSIGN, backup_proxy, result_proxy, RelocInfo::kNoPosition);
+  Expression* assignment2 = factory()->NewAssignment(
+      Token::ASSIGN, result_proxy, backup_proxy, RelocInfo::kNoPosition);
+  b->InsertStatementAt(
+      0, factory()->NewExpressionStatement(assignment1, RelocInfo::kNoPosition),
+      zone());
+  b->AddStatement(
+      factory()->NewExpressionStatement(assignment2, RelocInfo::kNoPosition),
+      zone());
+}
+
+
 void Processor::Process(ZoneList<Statement*>* statements) {
   for (int i = statements->length() - 1; i >= 0; --i) {
     Visit(statements->at(i));
@@ -167,9 +192,13 @@ void Processor::VisitTryCatchStatement(TryCatchStatement* node) {
 
 void Processor::VisitTryFinallyStatement(TryFinallyStatement* node) {
   // Rewrite both try and finally block (in reverse order).
+  bool set_after = is_set_;
+  is_set_ = true;  // Don't normally need to assign in finally block.
   Visit(node->finally_block());
   node->set_finally_block(replacement_->AsBlock());
-  Visit(node->try_block());  // Exception will not be caught.
+  BackupAndRestore(node->finally_block());
+  is_set_ = set_after;
+  Visit(node->try_block());
   node->set_try_block(replacement_->AsBlock());
   replacement_ = node;
 }
@@ -260,7 +289,8 @@ bool Rewriter::Rewrite(ParseInfo* info) {
         scope->NewTemporary(info->ast_value_factory()->dot_result_string());
     // The name string must be internalized at this point.
     DCHECK(!result->name().is_null());
-    Processor processor(info->isolate(), result, info->ast_value_factory());
+    Processor processor(info->isolate(), scope, result,
+                        info->ast_value_factory());
     processor.Process(body);
     if (processor.HasStackOverflow()) return false;