[interpreter] Minor clean-up of BytecodeSourceInfo.

src/interpreter/bytecode-pipeline.h

Index: src/interpreter/bytecode-pipeline.h
diff --git a/src/interpreter/bytecode-pipeline.h b/src/interpreter/bytecode-pipeline.h
index 3ac1a9e979fbcfec36f384d72546fdf81faaa7b0..0251e343a9c9d7317e1fb91d4c9bffb6e1ce5813 100644
--- a/src/interpreter/bytecode-pipeline.h
+++ b/src/interpreter/bytecode-pipeline.h
@@ -54,36 +54,85 @@ class BytecodeSourceInfo final {
  public:
   static const int kUninitializedPosition = -1;
 
-  BytecodeSourceInfo(int position = kUninitializedPosition,
-                     bool is_statement = false)
-      : source_position_(position), is_statement_(is_statement) {}
+  BytecodeSourceInfo()
+      : position_type_(PositionType::kNone),
+        source_position_(kUninitializedPosition) {}
+
+  BytecodeSourceInfo(int source_position, bool is_statement)
+      : position_type_(is_statement ? PositionType::kStatement
+                                    : PositionType::kExpression),
+        source_position_(source_position) {
+    DCHECK_GE(source_position, 0);
+  }
+
+  // Makes instance into a statement position.
+  void MakeStatementPosition(int source_position) {
+    // Statement positions can be replaced by other statement
+    // positions. For example , "for (x = 0; x < 3; ++x) 7;" has a
+    // statement position associated with 7 but no bytecode associated
+    // with it. Then Next is emitted after the body and has
+    // statement position and overrides the existing one.
+    position_type_ = PositionType::kStatement;
+    source_position_ = source_position;
+  }
+
+  // Makes instance into an expression position. Instance should not
+  // be a statement position otherwise it could be lost and impair the
+  // debugging experience.
+  void MakeExpressionPosition(int source_position) {
+    DCHECK(!is_statement());
+    position_type_ = PositionType::kExpression;
+    source_position_ = source_position;
+  }
+
+  // Forces an instance into an expression position.
+  void ForceExpressionPosition(int source_position) {
+    position_type_ = PositionType::kExpression;
+    source_position_ = source_position;
+  }
 
-  // Combine later source info with current.
-  void Update(const BytecodeSourceInfo& entry);
+  // Makes an invalid instance into a new source position.
+  void MakeInvalidEqualTo(const BytecodeSourceInfo& other) {

[rmcilroy, 2016/06/20 11:21:39]

I'm not keen on this name.

One possible suggestion to avoid the need for this function - how about in
BytecodeArrayBuilder, instead of having a AttachSourceInfo, have a
GetSourcePositionInfo function which returns either the current source position
or a invalid one, and pass it directly to the BytecodeNode constructor in
BytecodeArrayBuilder::Output? I think the compiler should optimize away the
extra construct/copy. WDYT?

Just noticed you have other uses of this outside BytecodeArrayBuilder. I think
the above is still possibly worth while, however I guess you still need this
function either way. As such, could we rename it to Clone() or something
similar?

[oth, 2016/06/21 10:05:46]

Okay, the method is renamed Clone(). I totally agree the existing name wasn't
great, was trying to convey overwriting uninitialized state.

GetSourcePositionInfo() is going to need the bytecode as an argument, not sure
it's a win either way. If you feel stronger about it, not problem with adding
it.

+    DCHECK(!is_valid());
+    position_type_ = other.position_type_;
+    source_position_ = other.source_position_;
+  }
 
   int source_position() const {
     DCHECK(is_valid());
     return source_position_;
   }
 
-  bool is_statement() const { return is_valid() && is_statement_; }
-  bool is_expression() const { return is_valid() && !is_statement_; }
+  bool is_statement() const {
+    return position_type_ == PositionType::kStatement;
+  }
+  bool is_expression() const {
+    return position_type_ == PositionType::kExpression;
+  }
 
-  bool is_valid() const { return source_position_ != kUninitializedPosition; }
-  void set_invalid() { source_position_ = kUninitializedPosition; }
+  bool is_valid() const { return position_type_ != PositionType::kNone; }
+  void set_invalid() {
+    position_type_ = PositionType::kNone;
+    source_position_ = kUninitializedPosition;
+  }
 
   bool operator==(const BytecodeSourceInfo& other) const {
-    return source_position_ == other.source_position_ &&
-           is_statement_ == other.is_statement_;
+    return position_type_ == other.position_type_ &&
+           source_position_ == other.source_position_;
   }
+
   bool operator!=(const BytecodeSourceInfo& other) const {
-    return source_position_ != other.source_position_ ||
-           is_statement_ != other.is_statement_;
+    return position_type_ != other.position_type_ ||
+           source_position_ != other.source_position_;
   }
 
  private:
+  enum class PositionType : uint8_t { kNone, kExpression, kStatement };
+
+  PositionType position_type_;
   int source_position_;
-  bool is_statement_;
+
+  DISALLOW_COPY_AND_ASSIGN(BytecodeSourceInfo);
 };
 
 // A container for a generated bytecode, it's operands, and source information.