[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) {

[vogelheim, 2016/02/29 14:52:32]

Remove literal parameter?

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

Done.

   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) {
+  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 14:52:32]

Is this ever supposed to happen?

+  if (exit_seen_in_block_) return;
+  source_position_table_builder_.AddStatementPosition(bytecodes_.size(),
+                                                      return_position_);

[vogelheim, 2016/02/29 14:52:32]

I'm confused at this logic. (Not specifically this part, but the general
idea...)

Is the idea to always set a return position at the end of the function, or is
the plan to set a return position to wherever returns are, but make sure there's
at least one (w/ end of the function being the default)?

If the former, we could just set if once & unconditionally when generating the
table.

If the latter, then I don't think this code does what it's meant to. This will
always set a statement position at the end (return_position_), regardless of
whether it's set via the first Return() call or is the default via
EnsureReturnPosition(). Also, I'd suspect in this case we'd want to set a
statement for each actual Return(), while this would set it only for the first
(since exit_seen_in_block_ will be true after the first Return().

 }
 
 void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {