warn about functions ending with a statement instead of a return value

src/script/parser.cpp

@@ -373,13 +373,23 @@ enum Precedence
 ,	PREC_NONE
 };
 
+enum ExprType
+{	EXPR_VAR			// A single variable, which could be converted to the left hand side of an assignment
+,	EXPR_STATEMENT		// A 'statement', i.e. an expression that shouldn't be the last one
+,	EXPR_OTHER
+,	EXPR_FAILED
+};
+
 /// Parse an expression
 /** @param input    Read tokens from the input
  *  @param scrip    Add resulting instructions to the script
  *  @param min_prec Minimum precedence level for operators
+ *  
+ *  @returns the type of expression
+ *  
  *  NOTE: The net stack effect of an expression should be +1
  */
-void parseExpr(TokenIterator& input, Script& script, Precedence min_prec);
+ExprType parseExpr(TokenIterator& input, Script& script, Precedence min_prec);
 
 /// Parse an expression, possibly with operators applied. Optionally adds an instruction at the end.
 /** @param input           Read tokens from the input
@@ -389,7 +399,7 @@ void parseExpr(TokenIterator& input, Script& script, Precedence min_prec);
  *  @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);
+ExprType 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);
@@ -434,9 +444,7 @@ bool expectToken(TokenIterator& input, const Char* expect, const Token* opening
 	}
 }
 
-void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
-	// usually loop only once, unless we encounter newlines
-	while (true) {
+ExprType parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 	Token token = input.read();
 	if (token == _("(")) {
 		// Parentheses = grouping for precedence of expressions
@@ -445,7 +453,10 @@ 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);
+		ExprType t = parseOper(input, *subScript, PREC_ALL);
+		if (t == EXPR_STATEMENT) {
+			input.add_error(_("Warning: last statement of a function should be an expression, i.e. it should return a result in all cases."));
+		}
 		expectToken(input, _("}"), &token);
 		script.addInstruction(I_PUSH_CONST, subScript);
 	} else if (token == _("[")) {
@@ -492,13 +503,15 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 			parseOper(input, script, PREC_SET);							// BBB
 			unsigned jmpEnd = script.addInstruction(I_JUMP);			//		jump lbl_end
 			script.comeFrom(jmpElse);									//		lbl_else:
-				if (input.peek() == _("else")) {							//else
+			bool has_else = input.peek() == _("else");					//else
+			if (has_else) {
 				input.read();
 				parseOper(input, script, PREC_SET);						// CCC
 			} else {
 				script.addInstruction(I_PUSH_CONST, script_nil);
 			}
 			script.comeFrom(jmpEnd);									//		lbl_end:
+			return has_else ? EXPR_OTHER : EXPR_STATEMENT;
 		} else if (token == _("for")) {
 			// the loop body should have a net stack effect of 0, but the entire expression of +1
 			// solution: add all results from the body, start with nil
@@ -616,10 +629,12 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 				script.addInstruction(I_NOP,  SCRIPT_VAR_condition);		// (condition:)
 				script.addInstruction(I_NOP,  SCRIPT_VAR_input);			// (input:)
 			}
+			return EXPR_STATEMENT;
 		} else {
 			// variable
 			Variable var = string_to_variable(token.value);
 			script.addInstruction(I_GET_VAR, var);
+			return EXPR_VAR;
 		}
 	} else if (token == TOK_INT) {
 		long l = 0;
@@ -634,17 +649,15 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 	} else if (token == TOK_STRING) {
 		script.addInstruction(I_PUSH_CONST, to_script(token.value));
 	} else {
+		script.addInstruction(I_PUSH_CONST, script_nil);
 		input.expected(_("expression"));
-			return;
-		}
-		break;
+		return EXPR_FAILED;
 	}
+	return EXPR_OTHER;
 }
 
-void parseOper(TokenIterator& input, Script& script, Precedence minPrec, InstructionType closeWith, int closeWithData) {
-	size_t added = script.getInstructions().size(); // number of instructions added
-	parseExpr(input, script, minPrec); // first argument
-	added = script.getInstructions().size() - added;
+ExprType parseOper(TokenIterator& input, Script& script, Precedence minPrec, InstructionType closeWith, int closeWithData) {
+	ExprType type = parseExpr(input, script, minPrec); // first argument
 	// read any operators after an expression
 	// EBNF:                    expr = expr | expr oper expr
 	// without left recursion:  expr = expr (oper expr)*
@@ -656,6 +669,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			input.putBack();
 			break;
 		}
+		
 		if (minPrec <= PREC_SEQ && token==_(";")) {
 			Token next = input.peek(1);
 			if (next == TOK_RPAREN || next == TOK_EOF) {
@@ -663,12 +677,12 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 				break;
 			}
 			script.addInstruction(I_POP); // discard result of first expression
-			parseOper(input, script, PREC_SET);
+			type = parseOper(input, script, PREC_SET);
 		} else if (minPrec <= PREC_SET && token==_(":=")) {
 			// We made a mistake, the part before the := should be a variable name,
 			// not an expression. Remove that instruction.
 			Instruction& instr = script.getInstructions().back();
-			if (added != 1 || instr.instr != I_GET_VAR) {
+			if (type != EXPR_VAR || instr.instr != I_GET_VAR) {
 				input.add_error(_("Can only assign to variables"));
 			}
 			script.getInstructions().pop_back();
@@ -790,16 +804,19 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			// only if we don't match another token!
 			input.putBack();
 			script.addInstruction(I_POP);
-			parseOper(input, script, PREC_SET);
+			type = parseOper(input, script, PREC_SET);
 		} else {
 			input.putBack();
 			break;
 		}
+		
+		if (type == EXPR_VAR) type = EXPR_OTHER; // var only applies to single variables, not to things with operators
 	}
 	// add closing instruction
 	if (closeWith != I_NOP) {
 		script.addInstruction(closeWith, closeWithData);
 	}
+	return type;
 }
 
 void parseCallArguments(TokenIterator& input, Script& script, vector<Variable>& arguments) {