[interpreter] Minor clean-up of BytecodeSourceInfo.

src/interpreter/bytecode-pipeline.h

 // Copyright 2015 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_BYTECODE_PIPELINE_H_
 #define V8_INTERPRETER_BYTECODE_PIPELINE_H_
 
 #include "src/interpreter/bytecode-register-allocator.h"
 #include "src/interpreter/bytecodes.h"
 #include "src/zone-containers.h"
@@ skipping 36 matching lines @@
   virtual Handle<BytecodeArray> ToBytecodeArray(
       int fixed_register_count, int parameter_count,
       Handle<FixedArray> handler_table) = 0;
 };
 
 // Source code position information.
 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) {}
 
-  // Combine later source info with current.
-  void Update(const BytecodeSourceInfo& entry);
+  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;
+  }
+
+  // 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.
 // These must be allocated by a BytecodeNodeAllocator instance.
 class BytecodeNode final : ZoneObject {
  public:
   explicit BytecodeNode(Bytecode bytecode = Bytecode::kIllegal);
   BytecodeNode(Bytecode bytecode, uint32_t operand0);
   BytecodeNode(Bytecode bytecode, uint32_t operand0, uint32_t operand1);
   BytecodeNode(Bytecode bytecode, uint32_t operand0, uint32_t operand1,
@@ skipping 44 matching lines @@
 };
 
 std::ostream& operator<<(std::ostream& os, const BytecodeSourceInfo& info);
 std::ostream& operator<<(std::ostream& os, const BytecodeNode& node);
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_INTERPRETER_BYTECODE_PIPELINE_H_