[parsing] Fix bug in completion value of try-finally.

src/parsing/rewriter.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parsing/rewriter.h"
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/parser.h"
@@ skipping 225 matching lines @@
 
   replacement_ = is_set_ && set_in_try ? node : AssignUndefinedBefore(node);
   is_set_ = true;
 }
 
 
 void Processor::VisitTryFinallyStatement(TryFinallyStatement* node) {
   // Only rewrite finally if it could contain 'break' or 'continue'. Always
   // rewrite try.
   if (breakable_) {
     bool set_after = is_set_;

[adamk, 2016/11/30 18:13:19]

Don't need this anymore

[neis, 2016/12/01 10:22:44]

Acknowledged.

     // Only set result before a 'break' or 'continue'.

[adamk, 2016/11/30 18:13:19]

This comment sounds redundant with the longer comment below. Maybe just drop the
second one?

[neis, 2016/12/01 10:22:44]

Not sure what you mean.

The comment here justifies why it's okay to set is_set_ to true before rewriting
the finally block.

The 4 line comment below talks about saving and restoring .result, I don't want
to remove that.

[adamk, 2016/12/01 18:49:31]

On re-reading them together, I think I agree that at least they're not
redundant.

     is_set_ = true;
     Visit(node->finally_block());
     node->set_finally_block(replacement_->AsBlock());
     // Save .result value at the beginning of the finally block and restore it
     // at the end again: ".backup = .result; ...; .result = .backup"
     // This is necessary because the finally block does not normally contribute
     // to the completion value.
     CHECK_NOT_NULL(closure_scope());
     Variable* backup = closure_scope()->NewTemporary(
         factory()->ast_value_factory()->dot_result_string());
     Expression* backup_proxy = factory()->NewVariableProxy(backup);
     Expression* result_proxy = factory()->NewVariableProxy(result_);
     Expression* save = factory()->NewAssignment(
         Token::ASSIGN, backup_proxy, result_proxy, kNoSourcePosition);
     Expression* restore = factory()->NewAssignment(
         Token::ASSIGN, result_proxy, backup_proxy, kNoSourcePosition);
     node->finally_block()->statements()->InsertAt(
         0, factory()->NewExpressionStatement(save, kNoSourcePosition), zone());
     node->finally_block()->statements()->Add(
         factory()->NewExpressionStatement(restore, kNoSourcePosition), zone());
-    is_set_ = set_after;
   }
   Visit(node->try_block());
   node->set_try_block(replacement_->AsBlock());
 
   replacement_ = is_set_ ? node : AssignUndefinedBefore(node);
   is_set_ = true;
 }
 
 
 void Processor::VisitSwitchStatement(SwitchStatement* node) {
@@ skipping 133 matching lines @@
           processor.SetResult(undef), expr->position());
       body->Add(completion, factory->zone());
     }
   }
   return true;
 }
 
 
 }  // namespace internal
 }  // namespace v8