[debugger] fix break locations for assignments and return.

src/interpreter/bytecode-array-builder.cc

Index: src/interpreter/bytecode-array-builder.cc
diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc
index 5bdf4ab24ba097ed5b017a661e2869a5d055c1df..c3899d0eee91dd5f82de782bf15f22ac236632d8 100644
--- a/src/interpreter/bytecode-array-builder.cc
+++ b/src/interpreter/bytecode-array-builder.cc
@@ -82,6 +82,7 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone,
       parameter_count_(parameter_count),
       local_register_count_(locals_count),
       context_register_count_(context_count),
+      return_position_(RelocInfo::kNoPosition),
       temporary_allocator_(zone, fixed_register_count()),
       register_translator_(this) {
   DCHECK_GE(parameter_count_, 0);
@@ -961,6 +962,7 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::ReThrow() {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Return() {
+  SetReturnPosition();
   Output(Bytecode::kReturn);
   exit_seen_in_block_ = true;
   return *this;
@@ -1045,7 +1047,6 @@ void BytecodeArrayBuilder::LeaveBasicBlock() {
 void BytecodeArrayBuilder::EnsureReturn(FunctionLiteral* literal) {
   if (!exit_seen_in_block_) {
     LoadUndefined();
-    SetReturnPosition(literal);
     Return();
   }
   DCHECK(exit_seen_in_block_);
@@ -1176,12 +1177,17 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
   return constant_array_builder()->Insert(object);
 }
 
-void BytecodeArrayBuilder::SetReturnPosition(FunctionLiteral* fun) {
-  // Don't emit dead code.
-  if (exit_seen_in_block_) return;
+void BytecodeArrayBuilder::InitializeReturnPosition(FunctionLiteral* literal) {

[rmcilroy, 2016/03/01 04:47:01]

I'm not particularly keen on this being a separate function. Could we instead
set it in the constructor? I would be fine with return_position_ being set to
kNoPosition if the literal passed to the constructor is nullptr if that would
help with tests. 

[Yang, 2016/03/01 08:09:18]

Done.

+  DCHECK_EQ(RelocInfo::kNoPosition, return_position_);
+  return_position_ =
+      std::max(literal->start_position(), literal->end_position() - 1);
+}
 
-  int pos = std::max(fun->start_position(), fun->end_position() - 1);
-  source_position_table_builder_.AddStatementPosition(bytecodes_.size(), pos);
+void BytecodeArrayBuilder::SetReturnPosition() {
+  if (return_position_ == RelocInfo::kNoPosition) return;

[vogelheim, 2016/02/29 17:43:30]

DCHECK? Or is this supposed to happen?

(Initialize is called from MakeBytecode, so I'd expect this condition to never
be true.)

[Yang, 2016/02/29 17:53:12]

I didnt want to add the initialize call to the about 100 uses of the bytecode
array builder in tests :p

+  if (exit_seen_in_block_) return;
+  source_position_table_builder_.AddStatementPosition(bytecodes_.size(),
+                                                      return_position_);
 }
 
 void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {