diff --git a/src/main/scala/millfork/compiler/AbstractStatementPreprocessor.scala b/src/main/scala/millfork/compiler/AbstractStatementPreprocessor.scala index dd29703e..1224e7ff 100644 --- a/src/main/scala/millfork/compiler/AbstractStatementPreprocessor.scala +++ b/src/main/scala/millfork/compiler/AbstractStatementPreprocessor.scala @@ -2,7 +2,6 @@ package millfork.compiler import millfork.CompilationFlag import millfork.env._ -import millfork.error.ConsoleLogger import millfork.node._ import AbstractExpressionCompiler.getExpressionType import millfork.compiler.AbstractStatementPreprocessor.hiddenEffectFreeFunctions @@ -60,6 +59,20 @@ abstract class AbstractStatementPreprocessor(ctx: CompilationContext, statements def optimizeStmt(stmt: ExecutableStatement, currentVarValues: VV): (ExecutableStatement, VV) = { var cv = currentVarValues val pos = stmt.position + // generic warnings: + stmt match { + case ExpressionStatement(expr@FunctionCallExpression("strzlen" | "putstrz", List(TextLiteralExpression(ch)))) => + ch.last match { + case LiteralExpression(0, _) => //ok + case _ => ctx.log.warn("Passing a non-null-terminated string to a function that expects a null-terminated string.", stmt.position) + } + case ExpressionStatement(VariableExpression(v)) => + val volatile = ctx.env.maybeGet[ThingInMemory](v).fold(false)(_.isVolatile) + if (!volatile) ctx.log.warn("Pointless expression.", stmt.position) + case ExpressionStatement(LiteralExpression(_, _)) => + ctx.log.warn("Pointless expression.", stmt.position) + case _ => + } stmt match { case Assignment(ve@VariableExpression(v), arg) if trackableVars(v) => cv = search(arg, cv) @@ -164,6 +177,17 @@ abstract class AbstractStatementPreprocessor(ctx: CompilationContext, statements def optimizeExpr(expr: Expression, currentVarValues: VV): Expression = { val pos = expr.position + // generic warnings: + expr match { + case FunctionCallExpression("*" | "*=", params) => + if (params.exists { + case LiteralExpression(0, _) => true + case _ => false + }) ctx.log.warn("Multiplication by zero.", params.head.position) + case FunctionCallExpression("<<" | ">>" | "<<'" | "<<=" | ">>=" | "<<'=" | ">>>>", List(lhs@_, LiteralExpression(0, _))) => + ctx.log.warn("Shift by zero.", lhs.position) + case _ => + } expr match { case FunctionCallExpression("->", List(handle, VariableExpression(field))) => expr diff --git a/src/main/scala/millfork/env/Environment.scala b/src/main/scala/millfork/env/Environment.scala index 550874a5..28f22320 100644 --- a/src/main/scala/millfork/env/Environment.scala +++ b/src/main/scala/millfork/env/Environment.scala @@ -712,6 +712,14 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa val w = get[Type]("word") val name = stmt.name val resultType = get[Type](stmt.resultType) + if (stmt.name == "main") { + if (stmt.resultType != "void") { + log.warn("`main` should return `void`.", stmt.position) + } + if (stmt.params.nonEmpty) { + log.warn("`main` shouldn't have parameters.", stmt.position) + } + } if (stmt.reentrant && stmt.interrupt) log.error(s"Reentrant function `$name` cannot be an interrupt handler", stmt.position) if (stmt.reentrant && stmt.params.nonEmpty) log.error(s"Reentrant function `$name` cannot have parameters", stmt.position) @@ -812,7 +820,7 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa env, stackVariablesSize, stmt.address.map(a => this.eval(a).getOrElse(errorConstant(s"Address of `${stmt.name}` is not a constant"))), - executableStatements ++ (if (needsExtraRTS) List(ReturnStatement(None)) else Nil), + executableStatements ++ (if (needsExtraRTS) List(ReturnStatement(None).pos(executableStatements.lastOption.fold(stmt.position)(_.position))) else Nil), interrupt = stmt.interrupt, kernalInterrupt = stmt.kernalInterrupt, reentrant = stmt.reentrant, diff --git a/src/test/scala/millfork/test/WarningSuite.scala b/src/test/scala/millfork/test/WarningSuite.scala new file mode 100644 index 00000000..b349eeaa --- /dev/null +++ b/src/test/scala/millfork/test/WarningSuite.scala @@ -0,0 +1,30 @@ +package millfork.test + +import millfork.Cpu +import millfork.test.emu.EmuUnoptimizedCrossPlatformRun +import org.scalatest.{FunSuite, Matchers} + +/** + * @author Karol Stasiak + */ +class WarningSuite extends FunSuite with Matchers { + + test("Various warnings") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80)( + """ + | void putstrz(pointer p) {} + | byte output@0xc000 + | byte main (byte x) { + | putstrz("a") + | byte a + | a + | 5 + | a *= 0 + | a <<= 0 + | output = 4 + | } + """.stripMargin) { m => + m.readByte(0xc000) should equal(4) + } + } +}