From c144d4e50106e66b08972f515c8dc8714472555e Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 20 Aug 2020 14:28:17 +0200 Subject: [PATCH] improved warnings about unreachable code --- .../src/prog8/ast/processing/AstChecker.kt | 26 ---------- .../src/prog8/ast/statements/AstStatements.kt | 9 ---- compiler/src/prog8/compiler/Main.kt | 3 +- .../compiler/target/c64/codegen/AsmGen.kt | 6 ++- .../src/prog8/optimizer/StatementOptimizer.kt | 2 +- .../src/prog8/optimizer/UnusedCodeRemover.kt | 39 +++++++++++--- examples/test.p8 | 52 ++++--------------- 7 files changed, 51 insertions(+), 86 deletions(-) diff --git a/compiler/src/prog8/ast/processing/AstChecker.kt b/compiler/src/prog8/ast/processing/AstChecker.kt index af5d1184f..70578be21 100644 --- a/compiler/src/prog8/ast/processing/AstChecker.kt +++ b/compiler/src/prog8/ast/processing/AstChecker.kt @@ -309,8 +309,6 @@ internal class AstChecker(private val program: Program, err("Pass-by-reference types (str, array) cannot occur as a parameter type directly. Instead, use an uword to receive their address, or access the variable from the outer scope directly.") } } - - visitStatements(subroutine.statements) } override fun visit(untilLoop: UntilLoop) { @@ -1027,30 +1025,6 @@ internal class AstChecker(private val program: Program, } } - override fun visit(scope: AnonymousScope) { - visitStatements(scope.statements) - } - - private fun visitStatements(statements: List) { - for((index, stmt) in statements.withIndex()) { - if(index < statements.lastIndex && statements[index+1] !is Subroutine) { - when { - stmt is FunctionCallStatement && stmt.target.nameInSource.last() == "exit" -> { - errors.warn("unreachable code, preceding exit call will never return", statements[index + 1].position) - } - stmt is Return && statements[index + 1] !is Subroutine -> { - errors.warn("unreachable code, preceding return statement", statements[index + 1].position) - } - stmt is Jump && statements[index + 1] !is Subroutine -> { - errors.warn("unreachable code, preceding jump statement", statements[index + 1].position) - } - } - } - - stmt.accept(this) - } - } - private fun checkFunctionOrLabelExists(target: IdentifierReference, statement: Statement): Statement? { val targetStatement = target.targetStatement(program.namespace) if(targetStatement is Label || targetStatement is Subroutine || targetStatement is BuiltinFunctionStatementPlaceholder) diff --git a/compiler/src/prog8/ast/statements/AstStatements.kt b/compiler/src/prog8/ast/statements/AstStatements.kt index 6a78c20ee..77686a5a9 100644 --- a/compiler/src/prog8/ast/statements/AstStatements.kt +++ b/compiler/src/prog8/ast/statements/AstStatements.kt @@ -139,15 +139,6 @@ open class Return(var value: Expression?, override val position: Position) : Sta } } -class ReturnFromIrq(override val position: Position) : Return(null, position) { - override fun accept(visitor: IAstVisitor) = visitor.visit(this) - - override fun toString(): String { - return "ReturnFromIrq(pos=$position)" - } - override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") -} - class Break(override val position: Position) : Statement() { override lateinit var parent: Node diff --git a/compiler/src/prog8/compiler/Main.kt b/compiler/src/prog8/compiler/Main.kt index f2979462f..0a59c6441 100644 --- a/compiler/src/prog8/compiler/Main.kt +++ b/compiler/src/prog8/compiler/Main.kt @@ -169,9 +169,10 @@ private fun optimizeAst(programAst: Program, errors: ErrorReporter) { break } - val remover = UnusedCodeRemover() + val remover = UnusedCodeRemover(errors) remover.visit(programAst) remover.applyModifications() + errors.handle() } private fun postprocessAst(programAst: Program, errors: ErrorReporter, compilerOptions: CompilationOptions) { diff --git a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt index bde350c2d..5ad7cdc21 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt @@ -636,7 +636,11 @@ internal class AsmGen(private val program: Program, is BranchStatement -> translate(stmt) is IfStatement -> translate(stmt) is ForLoop -> forloopsAsmGen.translate(stmt) - is Break -> out(" jmp ${loopEndLabels.peek()}") + is Break -> { + if(loopEndLabels.isEmpty()) + throw AssemblyError("break statement out of context ${stmt.position}") + out(" jmp ${loopEndLabels.peek()}") + } is WhileLoop -> translate(stmt) is RepeatLoop -> translate(stmt) is UntilLoop -> translate(stmt) diff --git a/compiler/src/prog8/optimizer/StatementOptimizer.kt b/compiler/src/prog8/optimizer/StatementOptimizer.kt index bd74538dd..7f6918f86 100644 --- a/compiler/src/prog8/optimizer/StatementOptimizer.kt +++ b/compiler/src/prog8/optimizer/StatementOptimizer.kt @@ -136,7 +136,7 @@ internal class StatementOptimizer(private val program: Program, val subroutine = functionCallStatement.target.targetSubroutine(program.namespace) if(subroutine!=null) { val first = subroutine.statements.asSequence().filterNot { it is VarDecl || it is Directive }.firstOrNull() - if(first is ReturnFromIrq || first is Return) + if(first is Return) return listOf(IAstModification.Remove(functionCallStatement, parent)) } diff --git a/compiler/src/prog8/optimizer/UnusedCodeRemover.kt b/compiler/src/prog8/optimizer/UnusedCodeRemover.kt index fb4b8c0c5..8c5e8ac63 100644 --- a/compiler/src/prog8/optimizer/UnusedCodeRemover.kt +++ b/compiler/src/prog8/optimizer/UnusedCodeRemover.kt @@ -1,16 +1,14 @@ package prog8.optimizer +import prog8.ast.INameScope import prog8.ast.Node import prog8.ast.Program +import prog8.ast.base.ErrorReporter import prog8.ast.processing.AstWalker import prog8.ast.processing.IAstModification -import prog8.ast.statements.Block +import prog8.ast.statements.* -/* - TODO: remove unreachable code after return and exit() -*/ - -internal class UnusedCodeRemover: AstWalker() { +internal class UnusedCodeRemover(private val errors: ErrorReporter): AstWalker() { override fun before(program: Program, parent: Node): Iterable { val callgraph = CallGraph(program) @@ -38,4 +36,33 @@ internal class UnusedCodeRemover: AstWalker() { return removals } + + + override fun before(breakStmt: Break, parent: Node): Iterable { + reportUnreachable(breakStmt, parent as INameScope) + return emptyList() + } + + override fun before(jump: Jump, parent: Node): Iterable { + reportUnreachable(jump, parent as INameScope) + return emptyList() + } + + override fun before(returnStmt: Return, parent: Node): Iterable { + reportUnreachable(returnStmt, parent as INameScope) + return emptyList() + } + + override fun before(functionCallStatement: FunctionCallStatement, parent: Node): Iterable { + if(functionCallStatement.target.nameInSource.last() == "exit") + reportUnreachable(functionCallStatement, parent as INameScope) + return emptyList() + } + + private fun reportUnreachable(stmt: Statement, parent: INameScope) { + when(val next = parent.nextSibling(stmt)) { + null, is Label, is Directive, is VarDecl, is InlineAssembly, is Subroutine, is StructDecl -> {} + else -> errors.warn("unreachable code", next.position) + } + } } diff --git a/examples/test.p8 b/examples/test.p8 index 9b380bbc7..d9e001861 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -7,53 +7,21 @@ main { sub start() { - struct Color { - ubyte red - uword green - float blue - } + c64scr.print_ub(5) + c64.CHROUT('\n') + return - Color c = [11,22222,3.1234] - - str string = "irmen" - byte[] ab = [1,2,3] - ubyte[] aub = [1,2,3] - word[] aw = [11,22,33] - uword[] auw = [11,22,33] - float[] af = [1.1,2.2,3.3] - - c64scr.print_ub(sizeof(ab)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(aub)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(aw)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(auw)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(af)) - c64.CHROUT('\n') - c64.CHROUT('\n') + c64scr.print_ub(5) c64.CHROUT('\n') - c64scr.print_ub(c.red) - c64.CHROUT('\n') - c64scr.print_uw(c.green) - c64.CHROUT('\n') - c64flt.print_f(c.blue) + goto start + + c64scr.print_ub(5) c64.CHROUT('\n') - ubyte size = sizeof(Color) - c64scr.print_ub(size) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(Color)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(c)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(c.red)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(c.green)) - c64.CHROUT('\n') - c64scr.print_ub(sizeof(c.blue)) + exit(11) + + c64scr.print_ub(5) c64.CHROUT('\n') } }