[interpreter] add source positions for call and call-new.

src/interpreter/source-position-table.h

Index: src/interpreter/source-position-table.h
diff --git a/src/interpreter/source-position-table.h b/src/interpreter/source-position-table.h
new file mode 100644
index 0000000000000000000000000000000000000000..b9cfaa6c0ed281f6ebe0199de6f6323fa2c16c02
--- /dev/null
+++ b/src/interpreter/source-position-table.h
@@ -0,0 +1,93 @@
+// Copyright 2016 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.
+
+#ifndef V8_INTERPRETER_SOURCE_POSITION_TABLE_H_
+#define V8_INTERPRETER_SOURCE_POSITION_TABLE_H_
+
+#include "src/assert-scope.h"
+#include "src/handles.h"
+#include "src/zone.h"
+#include "src/zone-containers.h"
+
+namespace v8 {
+namespace internal {
+
+class BytecodeArray;
+class FixedArray;
+class Isolate;
+
+namespace interpreter {
+
+class SourcePositionTableBase {

[vogelheim, 2016/02/03 11:42:09]

I find this an odd use of inheritance. Why not have a class
SourcePositionAndIsStatement [well, that's arguably an awkward name, but
whatever] that has a uint32_t member and int + bool accessors, and that does the
mapping using BitField<...> internally?

Btw.. this is totally a matter of personal preferences, so please proceed as you
see fit. :)

[rmcilroy, 2016/02/03 12:05:23]

+1

[Yang, 2016/02/03 14:16:20]

Done.

+ protected:
+  class IsStatementField : public BitField<bool, 0, 1> {};
+  class SourcePositionField : public BitField<unsigned, 1, 30> {};
+
+  uint32_t EncodeSourcePositionAndType(int source_position, bool is_statement) {
+    return IsStatementField::encode(is_statement) |
+           SourcePositionField::encode(source_position);
+  }
+};
+
+class SourcePositionTableBuilder : public SourcePositionTableBase {
+ public:
+  explicit SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
+      : isolate_(isolate), entries_(zone) {}
+
+  void AddStatementPosition(int bytecode_offset, int source_position) {
+    AssertMonotonic(bytecode_offset);
+    entries_.push_back({static_cast<uint32_t>(bytecode_offset),
+                        EncodeSourcePositionAndType(source_position, true)});

[rmcilroy, 2016/02/03 12:05:23]

I think these should really be in the cc file (I think the general advice is
that only setters / getters should be implmented in headers, although that may
be chromium advice that isn't followed in V8).

[Yang, 2016/02/03 14:16:20]

Done.

+  }
+
+  void AddExpressionPosition(int bytecode_offset, int source_position) {
+    AssertMonotonic(bytecode_offset);
+    entries_.push_back({static_cast<uint32_t>(bytecode_offset),
+                        EncodeSourcePositionAndType(source_position, false)});
+  }
+
+  Handle<FixedArray> ToFixedArray();
+
+ private:
+  struct Entry {
+    uint32_t bytecode_offset;
+    uint32_t source_position_and_type;
+  };
+
+  void AssertMonotonic(int bytecode_offset) {
+    DCHECK_IMPLIES(entries_.size() > 0,

[rmcilroy, 2016/02/03 12:05:23]

DHECK_IMPLIES got ripped out of chromium's base library for being confusing. I'd
suggest we should probably avoid using it for new things in V8 and possibly rip
it out of V8 in the future too for consistency.

[Yang, 2016/02/03 14:16:20]

Done.

+                   entries_.back().bytecode_offset < bytecode_offset);
+  }
+
+  Isolate* isolate_;
+  ZoneVector<Entry> entries_;
+
+  friend class SourcePositionTableReader;
+};
+
+class SourcePositionTableIterator : public SourcePositionTableBase {
+ public:
+  explicit SourcePositionTableIterator(BytecodeArray* bytecode_array);
+
+  static int PositionFromBytecodeOffset(BytecodeArray* bytecode_array,
+                                        int bytecode_offset);
+  bool Next();

[rmcilroy, 2016/02/03 12:05:24]

nit - most other v8 iterators seem to use Advance() and done() (e.g,
StackFrameIterator, BytecodeArrayIterator). I personally don't mind though -
feel free to keep it as is if you prefer this.

[Yang, 2016/02/03 14:16:20]

Done.

+  int bytecode_offset() const { return bytecode_offset_; }
+  int source_position() const { return source_position_; }
+  int is_statement() const { return is_statement_; }

[vogelheim, 2016/02/03 11:42:09]

bool?

is_statement_ is bool, so this is an implicit promtion.

[Yang, 2016/02/03 14:16:20]

Done. Nice catch. Thanks.

+
+ private:
+  FixedArray* table_;
+  int index_;
+  bool is_statement_;
+  int bytecode_offset_;
+  int source_position_;
+  DisallowHeapAllocation no_gc;
+};
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_INTERPRETER_SOURCE_POSITION_TABLE_H_