Dont lazily parse the ExpressionElement.

Instead of storing the raw bytecode string of expression elements
and then parsing them on demand, just parse the expression now
that the ExpressionPiece is cheap.

This similarly changes parts of the selection interface. (Also,
I don't see any uses of passing a non-default window number to
select_w in any games that I've disassembled locally.)

src/libreallive/bytecode.cc

@@ -366,41 +366,24 @@ void TextoutElement::RunOnMachine(RLMachine& machine) const {
 
 ExpressionElement::ExpressionElement(const char* src)
     : parsed_expression_(invalid_expression_piece_t()) {
-  // Don't parse the expression, just isolate it.
   const char* end = src;
-  end += NextToken(end);
-  if (*end == '\\') {
-    end += 2;
-    end += NextExpression(end);
-  }
-  repr.assign(src, end);
+  parsed_expression_ = GetAssignment(end);
+  length_ = std::distance(src, end);
 }
 
 ExpressionElement::ExpressionElement(const long val)
-    : parsed_expression_(invalid_expression_piece_t()) {
-  repr.resize(6, '$');
-  repr[1] = 0xff;
-  insert_i32(repr, 2, val);
+    : length_(0),
+      parsed_expression_(ExpressionPiece::IntConstant(val)) {
 }
 
 ExpressionElement::ExpressionElement(const ExpressionElement& rhs)
-    : parsed_expression_(rhs.parsed_expression_) {
+    : length_(0),
+      parsed_expression_(rhs.parsed_expression_) {
 }
 
 ExpressionElement::~ExpressionElement() {}
 
-int ExpressionElement::GetValueOnly(RLMachine& machine) const {
-  const char* location = repr.c_str();
-  ExpressionPiece e(GetExpression(location));
-  return e.GetIntegerValue(machine);
-}
-
 const ExpressionPiece& ExpressionElement::ParsedExpression() const {
-  if (!parsed_expression_.is_valid()) {
-    const char* location = repr.c_str();
-    parsed_expression_ = GetAssignment(location);
-  }
-
   return parsed_expression_;
 }
 
@@ -409,7 +392,7 @@ void ExpressionElement::PrintSourceRepresentation(std::ostream& oss) const {
 }
 
 const size_t ExpressionElement::GetBytecodeLength() const {
-  return repr.size();
+  return length_;
 }
 
 void ExpressionElement::RunOnMachine(RLMachine& machine) const {
@@ -554,9 +537,12 @@ SelectElement::SelectElement(const char* src)
 
 SelectElement::~SelectElement() {}
 
-ExpressionElement SelectElement::GetWindowExpression() const {
-  return repr[8] == '(' ? ExpressionElement(repr.data() + 9)
-                        : ExpressionElement(-1);
+ExpressionPiece SelectElement::GetWindowExpression() const {
+  if (repr[8] == '(') {
+    const char* location = repr.c_str() + 9;
+    return GetExpression(location);
+  }
+  return ExpressionPiece::IntConstant(-1);
 }
 
 const size_t SelectElement::GetParamCount() const { return params.size(); }

src/libreallive/bytecode.h

@@ -192,14 +192,7 @@ class ExpressionElement : public BytecodeElement {
   ExpressionElement(const ExpressionElement& rhs);
   virtual ~ExpressionElement();
 
-  // Assumes the expression isn't an assignment and returns the integer value.
-  //
-  // TODO(erg): We should be delete this if we make GetWindowExpression()
-  // return an ExpressionPiece instead.
-  int GetValueOnly(RLMachine& machine) const;
-
-  // Returns an ExpressionPiece representing this expression. This function
-  // lazily parses the expression and stores the tree for reuse.
+  // Returns an ExpressionPiece representing this expression.
   const ExpressionPiece& ParsedExpression() const;
 
   // Overridden from BytecodeElement:
@@ -208,11 +201,11 @@ class ExpressionElement : public BytecodeElement {
   virtual void RunOnMachine(RLMachine& machine) const final;
 
  private:
-  string repr;
+  int length_;
 
   // Storage for the parsed expression so we only have to calculate
   // it once (and so we can return it by const reference)
-  mutable ExpressionPiece parsed_expression_;
+  ExpressionPiece parsed_expression_;
 };
 
 // Command elements.
@@ -297,10 +290,7 @@ class SelectElement : public CommandElement {
 
   // Returns the expression in the source code which refers to which window to
   // display.
-  //
-  // TODO(erg): I believe I can simplify the only caller of this if I return an
-  // ExpressionPiece instead.
-  ExpressionElement GetWindowExpression() const;
+  ExpressionPiece GetWindowExpression() const;
 
   const params_t& raw_params() const { return params; }
 

src/modules/module_sel.cc

@@ -110,8 +110,8 @@ struct Sel_select_w : public RLOp_SpecialCase {
 
     // Sometimes the RL bytecode will override DEFAULT_SEL_WINDOW.
     int window = machine.system().gameexe()("DEFAULT_SEL_WINDOW").ToInt(-1);
-    libreallive::ExpressionElement window_exp = element.GetWindowExpression();
-    int computed = window_exp.GetValueOnly(machine);
+    libreallive::ExpressionPiece window_exp = element.GetWindowExpression();
+    int computed = window_exp.GetIntegerValue(machine);
     if (computed != -1)
       window = computed;