- fixed: Tacking on a return statement should only be done if the function has branches that actually reach the end. Otherwise it may interfere with return type deduction.

- used the return check to optimize out unneeded jumps at the end of an if statement's first block.

src/scripting/codegeneration/codegen.cpp

@@ -6879,6 +6879,18 @@ FxExpression *FxSequence::Resolve(FCompileContext &ctx)
 	return this;
 }
 
+//==========================================================================
+//
+// FxSequence :: CheckReturn
+//
+//==========================================================================
+
+bool FxSequence::CheckReturn()
+{
+	// a sequence always returns when its last element returns.
+	return Expressions.Size() > 0 && Expressions.Last()->CheckReturn();
+}
+
 //==========================================================================
 //
 // FxSequence :: Emit
@@ -7194,6 +7206,24 @@ ExpEmit FxSwitchStatement::Emit(VMFunctionBuilder *build)
 	return ExpEmit();
 }
 
+//==========================================================================
+//
+// FxSequence :: CheckReturn
+//
+//==========================================================================
+
+bool FxSwitchStatement::CheckReturn()
+{
+	//A switch statement returns when it contains no breaks and ends with a return
+	for (auto line : *Content)
+	{
+		if (line->ExprType == EFX_JumpStatement)
+		{
+			return false;	// Break means that the end of the statement will be reached, Continue cannot happen in the last statement of the last block.
+		}
+	}
+	return Content->Size() > 0 && Content->Last()->CheckReturn();
+}
 
 //==========================================================================
 //
@@ -7355,17 +7385,35 @@ ExpEmit FxIfStatement::Emit(VMFunctionBuilder *build)
 	v.Free(build);
 	if (path2 != nullptr)
 	{
-		size_t path1jump = build->Emit(OP_JMP, 0);
+		size_t path1jump;
+		
+		// if the branch ends with a return we do not need a terminating jmp.
+		if (!path1->CheckReturn()) path1jump = build->Emit(OP_JMP, 0);
+		else path1jump = 0xffffffff;
 		// Evaluate second path
 		build->BackpatchToHere(jumpspot);
 		v = path2->Emit(build);
 		v.Free(build);
 		jumpspot = path1jump;
 	}
-	build->BackpatchToHere(jumpspot);
+	if (jumpspot != 0xffffffff) build->BackpatchToHere(jumpspot);
 	return ExpEmit();
 }
 
+
+//==========================================================================
+//
+// FxIfStatement :: CheckReturn
+//
+//==========================================================================
+
+bool FxIfStatement::CheckReturn()
+{
+	//An if statement returns if both branches return, if present.
+	return (WhenTrue == nullptr || WhenTrue->CheckReturn()) &&
+			(WhenFalse == nullptr || WhenFalse->CheckReturn());
+}
+
 //==========================================================================
 //
 // FxLoopStatement :: Resolve

src/scripting/codegeneration/codegen.h

@@ -297,6 +297,7 @@ public:
 	virtual bool RequestAddress(bool *writable);
 	virtual PPrototype *ReturnProto();
 	virtual VMFunction *GetDirectFunction();
+	virtual bool CheckReturn() { return false; }
 	bool IsNumeric() const { return ValueType != TypeName && ValueType->GetRegCount() == 1 && (ValueType->GetRegType() == REGT_INT || ValueType->GetRegType() == REGT_FLOAT); }
 	bool IsFloat() const { return ValueType->GetRegType() == REGT_FLOAT && ValueType->GetRegCount() == 1; }
 	bool IsInteger() const { return ValueType != TypeName && (ValueType->GetRegType() == REGT_INT); }
@@ -1390,6 +1391,7 @@ public:
 	ExpEmit Emit(VMFunctionBuilder *build);
 	void Add(FxExpression *expr) { if (expr != NULL) Expressions.Push(expr); expr->NeedResult = false; }
 	VMFunction *GetDirectFunction();
+	bool CheckReturn();
 };
 
 //==========================================================================
@@ -1438,6 +1440,7 @@ public:
 	~FxSwitchStatement();
 	FxExpression *Resolve(FCompileContext&);
 	ExpEmit Emit(VMFunctionBuilder *build);
+	bool CheckReturn();
 };
 
 //==========================================================================
@@ -1476,6 +1479,7 @@ public:
 	~FxIfStatement();
 	FxExpression *Resolve(FCompileContext&);
 	ExpEmit Emit(VMFunctionBuilder *build);
+	bool CheckReturn();
 };
 
 //==========================================================================
@@ -1589,6 +1593,7 @@ public:
 	FxExpression *Resolve(FCompileContext&);
 	ExpEmit Emit(VMFunctionBuilder *build);
 	VMFunction *GetDirectFunction();
+	bool CheckReturn() { return true; }
 };
 
 //==========================================================================

src/scripting/vm/vmbuilder.cpp

@@ -715,6 +715,13 @@ void FFunctionBuildList::Build()
 
 		FScriptPosition::StrictErrors = !item.FromDecorate;
 		item.Code = item.Code->Resolve(ctx);
+		if (!item.Code->CheckReturn())
+		{
+			auto newcmpd = new FxCompoundStatement(item.Code->ScriptPosition);
+			newcmpd->Add(item.Code);
+			newcmpd->Add(new FxReturnStatement(nullptr, item.Code->ScriptPosition));
+			item.Code = newcmpd->Resolve(ctx);
+		}
 		item.Proto = ctx.ReturnProto;
 
 		// Make sure resolving it didn't obliterate it.

src/scripting/zscript/zcc_compile.cpp

@@ -2464,15 +2464,15 @@ FxExpression *ZCCCompiler::ConvertAST(PClass *cls, ZCC_TreeNode *ast)
 		// This must be done here so that we can check for a trailing return statement.
 		auto x = new FxCompoundStatement(*ast);
 		auto compound = static_cast<ZCC_CompoundStmt *>(ast);
-		bool isreturn = false;
+		//bool isreturn = false;
 		auto node = compound->Content;
 		if (node != nullptr) do
 		{
 			x->Add(ConvertNode(node));
-			isreturn = node->NodeType == AST_ReturnStmt;
+			//isreturn = node->NodeType == AST_ReturnStmt;
 			node = static_cast<decltype(node)>(node->SiblingNext);
 		} while (node != compound->Content);
-		if (!isreturn) x->Add(new FxReturnStatement(nullptr, *ast));
+		//if (!isreturn) x->Add(new FxReturnStatement(nullptr, *ast));
 		return x;
 	}
 }