Simplified script VM:

 - removed I_RET instruction, return is now implicit at end of script
 - I_POP is not a binary instruction.

src/data/field/choice.cpp

@@ -198,7 +198,6 @@ void ChoiceStyle::initImage() {
 	//       CALL       0
 	//       PUSH_CONST nil
 	//      OR_ELSE
-	//     RET
 	intrusive_ptr<ScriptCustomCollection> lookup(new ScriptCustomCollection());
 	FOR_EACH(ci, choice_images) {
 		lookup->key_value[ci.first] = ci.second.getScriptP();
@@ -210,7 +209,6 @@ void ChoiceStyle::initImage() {
 	script.addInstruction(I_CALL,       0);
 	script.addInstruction(I_PUSH_CONST, script_nil);
 	script.addInstruction(I_BINARY,     I_OR_ELSE);
-	script.addInstruction(I_RET);
 }
 
 int ChoiceStyle::update(Context& ctx) {

src/data/game.cpp

@@ -99,7 +99,6 @@ void Game::initCardListColorScript() {
 			s.addInstruction(I_BINARY,     I_MEMBER);
 			s.addInstruction(I_PUSH_CONST, to_script(Color(0,0,0)));
 			s.addInstruction(I_BINARY,     I_OR_ELSE);
-			s.addInstruction(I_RET);
 			return;
 		}
 	}

src/data/statistics.cpp

@@ -56,13 +56,11 @@ StatsDimension::StatsDimension(const Field& field)
 		s.addInstruction(I_MEMBER_C,   field.name);
 		s.addInstruction(I_CALL,       1);
 		s.addInstruction(I_NOP,        SCRIPT_VAR_input);
-		s.addInstruction(I_RET);
 	} else {
 		// initialize script, card.{field_name}
 		Script& s = script.getScript();
 		s.addInstruction(I_GET_VAR,    SCRIPT_VAR_card);
 		s.addInstruction(I_MEMBER_C,   field.name);
-		s.addInstruction(I_RET);
 	}
 }
 

src/script/context.cpp

@@ -39,12 +39,10 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 	try {
 		// Instruction pointer
 		const Instruction* instr = &script.instructions[0];
+		const Instruction* end   = &*script.instructions.end();
 		
 		// Loop until we are done
-		while (true) {
-			assert(instr < &*script.instructions.end());
-			// debug
-//			cout << script.dumpInstr(instr - &script.instructions[0], *instr) << endl;
+		while (instr < end) {
 			// Evaluate the current instruction
 			Instruction i = *instr++;
 			switch (i.instr) {
@@ -54,11 +52,6 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 					stack.push_back(script.constants[i.data]);
 					break;
 				}
-				// Pop top value
-				case I_POP: {
-					stack.pop_back();
-					break;
-				}
 				// Jump
 				case I_JUMP: {
 					instr = &script.instructions[i.data];
@@ -144,21 +137,10 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 					closeScope(scope);
 					break;
 				}
-				// Function return
-				case I_RET: {
-					// restore shadowed variables
-					if (useScope) closeScope(scope);
-					// return top of stack
-					ScriptValueP result = stack.back();
-					stack.pop_back();
-					assert(stack.size() == stack_size); // we end up with the same stack
-					return result;
-				}
 				
 				// Simple instruction: unary
 				case I_UNARY: {
 					instrUnary(i.instr1, stack.back());
-//					cout << "\t\t-> " << (String)*stack.back() << endl;
 					break;
 				}
 				// Simple instruction: binary
@@ -166,7 +148,6 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 					ScriptValueP  b = stack.back(); stack.pop_back();
 					ScriptValueP& a = stack.back();
 					instrBinary(i.instr2, a, b);
-//					cout << "\t\t-> " << (String)*stack.back() << endl;
 					break;
 				}
 				// Simple instruction: ternary
@@ -175,7 +156,6 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 					ScriptValueP  b = stack.back(); stack.pop_back();
 					ScriptValueP& a = stack.back();
 					instrTernary(i.instr3, a, b, c);
-//					cout << "\t\t-> " << (String)*stack.back() << endl;
 					break;
 				}
 				// Simple instruction: quaternary
@@ -185,12 +165,20 @@ ScriptValueP Context::eval(const Script& script, bool useScope) {
 					ScriptValueP  b = stack.back(); stack.pop_back();
 					ScriptValueP& a = stack.back();
 					instrQuaternary(i.instr4, a, b, c, d);
-//					cout << "\t\t-> " << (String)*stack.back() << endl;
 					break;
 				}
 			}
 		}
 		
+		// Function return
+		// restore shadowed variables
+		if (useScope) closeScope(scope);
+		// return top of stack
+		ScriptValueP result = stack.back();
+		stack.pop_back();
+		assert(stack.size() == stack_size); // we end up with the same stack
+		return result;
+		
 	} catch (...) {
 		// cleanup after an exception
 		if (useScope) closeScope(scope); // restore scope
@@ -322,14 +310,19 @@ class ScriptCompose : public ScriptValue {
 };
 
 void instrBinary (BinaryInstructionType  i, ScriptValueP& a, const ScriptValueP& b) {
-	ScriptType at = a->type(), bt = b->type();
 	switch (i) {
+		case I_POP:
+			// a = a;
+			break;
 		case I_MEMBER:
 			a = a->getMember(*b);
 			break;
 		case I_ITERATOR_R:
 			a = rangeIterator(*a, *b);
 			break;
+		default:
+	  ScriptType at = a->type(), bt = b->type();
+	  switch(i) {
 		case I_ADD: // add is quite overloaded
 			if (at == SCRIPT_NIL) {
 				a = b;
@@ -370,7 +363,7 @@ void instrBinary (BinaryInstructionType  i, ScriptValueP& a, const ScriptValueP&
 		case I_OR_ELSE:
 			if (at == SCRIPT_ERROR) a = b;
 			break;
-	}
+	}}
 }
 
 // ----------------------------------------------------------------------------- : Simple instructions : ternary

src/script/dependency.cpp

@@ -140,7 +140,6 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 		
 		// Loop until we are done
 		while (true) {
-			assert(instr < &*script.instructions.end());
 			// Is there a jump going here?
 			// If so, unify with current execution path
 			while (!jumps.empty() && jumps.top()->target == instr) {
@@ -158,6 +157,8 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 				delete j;
 			}
 			
+			if (instr >= &*script.instructions.end()) break; // end of script
+			
 			// Analyze the current instruction
 			Instruction i = *instr++;
 			switch (i.instr) {
@@ -167,11 +168,6 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 					stack.push_back(script.constants[i.data]);
 					break;
 				}
-				// Pop top value (as normal)
-				case I_POP: {
-					stack.pop_back();
-					break;
-				}
 				// Jump
 				case I_JUMP: {
 					if (&script.instructions[i.data] >= instr) {
@@ -253,16 +249,6 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 					closeScope(scope);
 					break;
 				}
-				// Function return (as normal)
-				case I_RET: {
-					closeScope(scope);
-					// return top of stack
-					ScriptValueP result = stack.back();
-					stack.pop_back();
-					assert(stack.size() == stack_size); // we end up with the same stack
-					assert(jumps.empty());              // no open jump records
-					return result;
-				}
 				
 				// Get a variable (almost as normal)
 				case I_GET_VAR: {
@@ -293,6 +279,10 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 				}
 				// Simple instruction: binary
 				case I_BINARY: {
+					if (i.instr2 == I_POP) {
+						stack.pop_back();
+						continue;
+					}
 					ScriptValueP  b = stack.back(); stack.pop_back();
 					ScriptValueP& a = stack.back();
 					switch (i.instr2) {
@@ -330,6 +320,15 @@ ScriptValueP Context::dependencies(const Dependency& dep, const Script& script)
 				}
 			}
 		}
+		
+		// Function return (as normal)
+		closeScope(scope);
+		ScriptValueP result = stack.back();
+		stack.pop_back();
+		assert(stack.size() == stack_size); // we end up with the same stack
+		assert(jumps.empty());              // no open jump records
+		return result;
+		
 	} catch (...) {
 		// cleanup after an exception
 		// the only place where exceptions should be possible is in someValue->getMember

src/script/image.cpp

@@ -82,7 +82,6 @@ ScriptP ScriptableImage::getScriptP() {
 	// return value or a blank image
 	ScriptP s(new Script);
 	s->addInstruction(I_PUSH_CONST, value ? static_pointer_cast<ScriptValue>(value) : script_nil);
-	s->addInstruction(I_RET);
 	return s;
 }
 

src/script/parser.cpp

@@ -346,19 +346,22 @@ void parseExpr(TokenIterator& input, Script& script, Precedence min_prec);
 /** @param input           Read tokens from the input
  *  @param script          Add resulting instructions to the script
  *  @param min_prec        Minimum precedence level for operators
- *  @param close_with      Add this instruction at the end
+ *  @param close_with      Add this instruction at the end, or I_NOP for no instruction
  *  @param close_with_data Data for the instruction at the end
  *  NOTE: The net stack effect of an expression should be +1
  */
 void parseOper(TokenIterator& input, Script& script, Precedence min_prec, InstructionType close_with = I_NOP, int close_with_data = 0);
 
+/// Parse call arguments, "(...)"
+void parseCallArguments(TokenIterator& input, Script& script, vector<Variable>& arguments);
+
 
 ScriptP parse(const String& s, Packaged* package, bool string_mode, vector<ScriptParseError>& errors_out) {
 	errors_out.clear();
 	// parse
 	TokenIterator input(s, package, string_mode, errors_out);
 	ScriptP script(new Script);
-	parseOper(input, *script, PREC_ALL, I_RET);
+	parseOper(input, *script, PREC_ALL);
 	Token eof = input.read();
 	if (eof != TOK_EOF) {
 		input.expected(_("end of input"));
@@ -403,7 +406,7 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 		} else if (token == _("{")) {
 			// {} = function block. Parse a new Script
 			intrusive_ptr<Script> subScript(new Script);
-			parseOper(input, *subScript, PREC_ALL, I_RET);
+			parseOper(input, *subScript, PREC_ALL);
 			expectToken(input, _("}"));
 			script.addInstruction(I_PUSH_CONST, subScript);
 		} else if (token == _("[")) {
@@ -447,11 +450,11 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 				unsigned int jmpElse, jmpEnd;
 				parseOper(input, script, PREC_AND);						// AAA
 				jmpElse = script.getLabel();							//		jmp_else:
-				script.addInstruction(I_JUMP_IF_NOT, 0xFFFFFFFF);		//		jnz lbl_else
+				script.addInstruction(I_JUMP_IF_NOT, INVALID_ADDRESS);	//		jnz lbl_else
 				expectToken(input, _("then"));							// then
 				parseOper(input, script, PREC_SET);						// BBB
 				jmpEnd = script.getLabel();								//		jmp_end:
-				script.addInstruction(I_JUMP, 0xFFFFFFFF);				//		jump lbl_end
+				script.addInstruction(I_JUMP, INVALID_ADDRESS);			//		jump lbl_end
 				script.comeFrom(jmpElse);								//		lbl_else:
 				if (input.peek() == _("else")) {						// else
 					input.read();
@@ -476,11 +479,11 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 					script.addInstruction(I_UNARY, I_ITERATOR_C);		//		iterator_collection
 					script.addInstruction(I_PUSH_CONST, script_nil);	//		push nil
 					lblStart = script.getLabel();						//		lbl_start:
-					script.addInstruction(I_LOOP, 0xFFFFFFFF);			//		loop
+					script.addInstruction(I_LOOP, INVALID_ADDRESS);		//		loop
 					expectToken(input, _("do"));						// do
 					script.addInstruction(I_SET_VAR,
 										string_to_variable(name.value));//		set name
-					script.addInstruction(I_POP);						//		 pop
+					script.addInstruction(I_BINARY, I_POP);				//		 pop
 					parseOper(input, script, PREC_SET, I_BINARY, I_ADD);// CCC;	add
 					script.addInstruction(I_JUMP, lblStart);			//		jump lbl_start
 					script.comeFrom(lblStart);							//		lbl_end:
@@ -494,11 +497,11 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 					script.addInstruction(I_BINARY, I_ITERATOR_R);		//		iterator_range
 					script.addInstruction(I_PUSH_CONST, script_nil);	//		push nil
 					lblStart = script.getLabel();						//		lbl_start:
-					script.addInstruction(I_LOOP, 0xFFFFFFFF);			//		loop
+					script.addInstruction(I_LOOP, INVALID_ADDRESS);		//		loop
 					expectToken(input, _("do"));						// do
 					script.addInstruction(I_SET_VAR,
 										string_to_variable(name.value));//		set name
-					script.addInstruction(I_POP);						//		 pop
+					script.addInstruction(I_BINARY, I_POP);				//		 pop
 					parseOper(input, script, PREC_SET, I_BINARY, I_ADD);// DDD;	add
 					script.addInstruction(I_JUMP, lblStart);			//		jump lbl_start
 					script.comeFrom(lblStart);							//		lbl_end:
@@ -582,7 +585,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 				// allow ; at end of expression without errors
 				break;
 			}
-			script.addInstruction(I_POP); // discard result of first expression
+			script.addInstruction(I_BINARY, I_POP); // discard result of first expression
 			parseOper(input, script, PREC_SET);
 		} else if (minPrec <= PREC_SET && token==_(":=")) {
 			// We made a mistake, the part before the := should be a variable name,
@@ -646,27 +649,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 		} else if (minPrec <= PREC_FUN && token==_("(")) {
 			// function call, read arguments
 			vector<Variable> arguments;
-			Token t = input.peek();
-			while (t != _(")") && t != TOK_EOF) {
-				if (input.peek(2) == _(":") && t.type == TOK_NAME) {
-					// name: ...
-					arguments.push_back(string_to_variable(t.value));
-					input.read(); // skip the name
-					input.read(); // and the :
-					parseOper(input, script, PREC_SEQ);
-				} else {
-					// implicit "input" argument
-					arguments.push_back(SCRIPT_VAR_input);
-					parseOper(input, script, PREC_SEQ);
-				}
-				t = input.peek();
-				if (t == _(",")) {
-					// Comma separating the arguments
-					input.read();
-					t = input.peek();
-				}
-			}
-			expectToken(input, _(")"));
+			parseCallArguments(input, script, arguments);
 			// generate instruction
 			script.addInstruction(I_CALL, (unsigned int)arguments.size());
 			FOR_EACH(arg,arguments) {
@@ -696,7 +679,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			// newline functions as ;
 			// only if we don't match another token!
 			input.putBack();
-			script.addInstruction(I_POP);
+			script.addInstruction(I_BINARY, I_POP);
 			parseOper(input, script, PREC_SET);
 		} else {
 			input.putBack();
@@ -708,3 +691,27 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 		script.addInstruction(closeWith, closeWithData);
 	}
 }
+
+void parseCallArguments(TokenIterator& input, Script& script, vector<Variable>& arguments) {
+	Token t = input.peek();
+	while (t != _(")") && t != TOK_EOF) {
+		if (input.peek(2) == _(":") && t.type == TOK_NAME) {
+			// name: ...
+			arguments.push_back(string_to_variable(t.value));
+			input.read(); // skip the name
+			input.read(); // and the :
+			parseOper(input, script, PREC_SEQ);
+		} else {
+			// implicit "input" argument
+			arguments.push_back(SCRIPT_VAR_input);
+			parseOper(input, script, PREC_SEQ);
+		}
+		t = input.peek();
+		if (t == _(",")) {
+			// Comma separating the arguments
+			input.read();
+			t = input.peek();
+		}
+	}
+	expectToken(input, _(")"));
+}

src/script/script.cpp

@@ -118,6 +118,7 @@ void Script::comeFrom(unsigned int pos) {
 	assert( instructions.at(pos).instr == I_JUMP
 	     || instructions.at(pos).instr == I_JUMP_IF_NOT
 	     || instructions.at(pos).instr == I_LOOP);
+	assert( instructions.at(pos).data == INVALID_ADDRESS );
 	instructions.at(pos).data = (unsigned int)instructions.size();
 }
 
@@ -145,7 +146,6 @@ String Script::dumpInstr(unsigned int pos, Instruction i) const {
 	switch (i.instr) {
 		case I_NOP:			ret += _("nop");		break;
 		case I_PUSH_CONST:	ret += _("push");		break;
-		case I_POP:			ret += _("pop");		break;
 		case I_JUMP:		ret += _("jump");		break;
 		case I_JUMP_IF_NOT:	ret += _("jnz");		break;
 		case I_GET_VAR:		ret += _("get");		break;
@@ -154,7 +154,6 @@ String Script::dumpInstr(unsigned int pos, Instruction i) const {
 		case I_LOOP:		ret += _("loop");		break;
 		case I_MAKE_OBJECT:	ret += _("make object");break;
 		case I_CALL:		ret += _("call");		break;
-		case I_RET:			ret += _("ret");		break;
 		case I_UNARY:		ret += _("unary\t");
 			switch (i.instr1) {
 				case I_ITERATOR_C:	ret += _("iterator_c");	break;
@@ -164,6 +163,7 @@ String Script::dumpInstr(unsigned int pos, Instruction i) const {
 			break;
 		case I_BINARY:		ret += _("binary\t");
 			switch (i.instr2) {
+				case I_POP:			ret += _("pop");		break;
 				case I_ITERATOR_R:	ret += _("iterator_r");	break;
 				case I_MEMBER:		ret += _("member");		break;
 				case I_ADD:			ret += _("+");			break;
@@ -229,7 +229,6 @@ const Instruction* Script::backtraceSkip(const Instruction* instr, int to_skip)
 			case I_PUSH_CONST:
 			case I_GET_VAR:
 				to_skip -= 1; break; // nett stack effect +1
-			case I_POP:
 			case I_BINARY:
 				to_skip += 1; break; // nett stack effect 1-2 == -1
 			case I_TERNARY:
@@ -282,7 +281,7 @@ const Instruction* Script::backtraceSkip(const Instruction* instr, int to_skip)
 				++instr; // compensate for the -- in the outer loop
 				break;
 			}
-			case I_RET: case I_JUMP_IF_NOT: case I_LOOP:
+			case I_JUMP_IF_NOT: case I_LOOP:
 				return nullptr; // give up
 			default:
 				break; // nett stack effect 0

src/script/script.hpp

@@ -21,27 +21,25 @@ DECLARE_POINTER_TYPE(Script);
  */
 enum InstructionType
 	// Basic
-{	I_NOP			= 0  ///< no operation, used as placeholder for extra data values
-,	I_PUSH_CONST	= 1  ///< arg = value : push a constant onto the stack
-,	I_POP			= 2  ///< pop the top value from the stack (used for ; operator)
-,	I_JUMP			= 3  ///< arg = int   : move the instruction pointer to the given position
-,	I_JUMP_IF_NOT	= 4  ///< arg = int   : move the instruction pointer if the top of the stack is false
+{	I_NOP			= 0  ///< arg = *          : no operation, used as placeholder for extra data values
+,	I_PUSH_CONST	= 1  ///< arg = const val  : push a constant onto the stack
+,	I_JUMP			= 2  ///< arg = address    : move the instruction pointer to the given position
+,	I_JUMP_IF_NOT	= 3  ///< arg = address    : move the instruction pointer if the top of the stack is false
 	// Variables
-,	I_GET_VAR		= 5  ///< arg = int   : find a variable, push its value onto the stack, it is an error if the variable is not found
-,	I_SET_VAR		= 6  ///< arg = int   : assign the top value from the stack to a variable (doesn't pop)
+,	I_GET_VAR		= 4  ///< arg = var        : find a variable, push its value onto the stack, it is an error if the variable is not found
+,	I_SET_VAR		= 5  ///< arg = var        : assign the top value from the stack to a variable (doesn't pop)
 	// Objects
-,	I_MEMBER_C		= 7  ///< arg = name  : finds a member of the top of the stack replaces the top of the stack with the member
-,	I_LOOP			= -8 ///< arg = int   : loop over the elements of an iterator, which is the *second* element of the stack (this allows for combing the results of multiple iterations)
-					     ///<               at the end performs a jump and pops the iterator. note: The second element of the stack must be an iterator!
-,	I_MAKE_OBJECT   = -7 ///< arg = int   : make a list/map with n elements, pops 2n values of the stack, n key/value pairs
+,	I_MEMBER_C		= 6  ///< arg = const name : finds a member of the top of the stack replaces the top of the stack with the member
+,	I_LOOP			= 7  ///< arg = address    : loop over the elements of an iterator, which is the *second* element of the stack (this allows for combing the results of multiple iterations)
+					     ///<                    at the end performs a jump and pops the iterator. note: The second element of the stack must be an iterator!
+,	I_MAKE_OBJECT   = 8  ///< arg = int        : make a list/map with n elements, pops 2n values of the stack, n key/value pairs
 	// Functions
-,	I_CALL			= -6 ///< arg = int, int+ : call the top item of the stack, with the given number of arguments (set with SET_VAR, but in the activation record of the call)
-,	I_RET			= -5 ///< return from the current function
+,	I_CALL			= 9  ///< arg = int, n*var : call the top item of the stack, with the given number of arguments (set with SET_VAR, but in the activation record of the call)
 	// Simple instructions
-,	I_UNARY			= -4 ///< pop 1 value,  apply a function, push the result
-,	I_BINARY		= -3 ///< pop 2 values, apply a function, push the result
-,	I_TERNARY		= -2 ///< pop 3 values, apply a function, push the result
-,	I_QUATERNARY	= -1 ///< pop 4 values, apply a function, push the result
+,	I_UNARY			= 10 ///< arg = 1ary instr : pop 1 value,  apply a function, push the result
+,	I_BINARY		= 11 ///< arg = 2ary instr : pop 2 values, apply a function, push the result
+,	I_TERNARY		= 12 ///< arg = 3ary instr : pop 3 values, apply a function, push the result
+,	I_QUATERNARY	= 13 ///< arg = 4ary instr : pop 4 values, apply a function, push the result
 };
 
 /// Types of unary instructions (taking one argument from the stack)
@@ -53,7 +51,8 @@ enum UnaryInstructionType
 
 /// Types of binary instructions (taking two arguments from the stack)
 enum BinaryInstructionType
-{	I_ITERATOR_R	///< Make an iterator for a range (two integers)
+{	I_POP			///< Pop the top value of the stack
+,	I_ITERATOR_R	///< Make an iterator for a range (two integers)
 ,	I_MEMBER		///< Member of an object
 // Arithmatic
 ,	I_ADD			///< add     
@@ -93,15 +92,16 @@ enum QuaternaryInstructionType
  *  Then the instr? member gives the actual instruction to perform
  */
 struct Instruction {
-	InstructionType instr : 4;
+	InstructionType instr : 5;
 	union {
-		unsigned int				data   : 28;
-		UnaryInstructionType		instr1 : 28;
-		BinaryInstructionType		instr2 : 28;
-		TernaryInstructionType		instr3 : 28;
-		QuaternaryInstructionType	instr4 : 28;
+		unsigned int				data   : 27;
+		UnaryInstructionType		instr1 : 27;
+		BinaryInstructionType		instr2 : 27;
+		TernaryInstructionType		instr3 : 27;
+		QuaternaryInstructionType	instr4 : 27;
 	};
 };
+static const unsigned int INVALID_ADDRESS = 0x03FFFFFF;
 
 // ----------------------------------------------------------------------------- : Variables