From b1321e40090dc78d9fb1c71088ee8f79eb2cd874 Mon Sep 17 00:00:00 2001
From: Elliot Glaysher <glaysher@umich.edu>
Date: Sat, 2 May 2015 15:30:27 -0700
Subject: [PATCH] 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 | 40 ++++++++++++-------------------------
 src/libreallive/bytecode.h  | 18 ++++-------------
 src/modules/module_sel.cc   |  4 ++--
 3 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/src/libreallive/bytecode.cc b/src/libreallive/bytecode.cc
index 937a957b3..4b1c0504d 100644
--- a/src/libreallive/bytecode.cc
+++ b/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(); }
diff --git a/src/libreallive/bytecode.h b/src/libreallive/bytecode.h
index ea1167848..24e1ac3e0 100644
--- a/src/libreallive/bytecode.h
+++ b/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; }
 
diff --git a/src/modules/module_sel.cc b/src/modules/module_sel.cc
index 946e02753..60c63200a 100644
--- a/src/modules/module_sel.cc
+++ b/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;