From ff35ba369615a805f5050cdf259ae09b3601155e Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 30 Jul 2023 17:10:54 +0200 Subject: [PATCH] added warnshadow cli option to enable assembler warnings about symbol shadowing --- codeCore/src/prog8/code/core/CompilationOptions.kt | 1 + .../src/prog8/codegen/cpu6502/AssemblyProgram.kt | 12 +++++++++++- compiler/src/prog8/CompilerMain.kt | 3 +++ compiler/src/prog8/compiler/Compiler.kt | 2 ++ compiler/test/TestCompilerOnExamples.kt | 1 + compiler/test/TestCompilerOptionLibdirs.kt | 1 + compiler/test/helpers/compileXyz.kt | 1 + docs/source/compiling.rst | 9 +++++++++ docs/source/todo.rst | 4 +++- examples/test.p8 | 10 ++++++---- httpCompilerService/src/prog8/http/TestHttp.kt | 1 + 11 files changed, 39 insertions(+), 6 deletions(-) diff --git a/codeCore/src/prog8/code/core/CompilationOptions.kt b/codeCore/src/prog8/code/core/CompilationOptions.kt index bda16934e..b7e7e20a0 100644 --- a/codeCore/src/prog8/code/core/CompilationOptions.kt +++ b/codeCore/src/prog8/code/core/CompilationOptions.kt @@ -13,6 +13,7 @@ class CompilationOptions(val output: OutputType, val compTarget: ICompilationTarget, // these are set later, based on command line arguments or options in the source code: var loadAddress: UInt, + var warnSymbolShadowing: Boolean = false, var optimize: Boolean = false, var asmQuiet: Boolean = false, var asmListfile: Boolean = false, diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AssemblyProgram.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AssemblyProgram.kt index 9d90b841f..1c1e8dccb 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AssemblyProgram.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AssemblyProgram.kt @@ -31,6 +31,11 @@ internal class AssemblyProgram( "--dump-labels", "--vice-labels", "--labels=$viceMonListFile", "--no-monitor" ) + if(options.warnSymbolShadowing) + command.add("-Wshadow") + else + command.add("-Wno-shadow") + if(options.asmQuiet) command.add("--quiet") @@ -59,10 +64,15 @@ internal class AssemblyProgram( // TODO are these options okay for atari? val command = mutableListOf("64tass", "--ascii", "--case-sensitive", "--long-branch", - "-Wall", "-Wno-strict-bool", "-Wno-shadow", // "-Werror", + "-Wall", // "-Werror", "-Wno-strict-bool" "--no-monitor" ) + if(options.warnSymbolShadowing) + command.add("-Wshadow") + else + command.add("-Wno-shadow") + if(options.asmQuiet) command.add("--quiet") diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 17bc89064..cb41a2b47 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -45,6 +45,7 @@ private fun compileMain(args: Array): Boolean { val dontOptimize by cli.option(ArgType.Boolean, fullName = "noopt", description = "don't perform any optimizations") val outputDir by cli.option(ArgType.String, fullName = "out", description = "directory for output files instead of current directory").default(".") val quietAssembler by cli.option(ArgType.Boolean, fullName = "quietasm", description = "don't print assembler output results") + val warnSymbolShadowing by cli.option(ArgType.Boolean, fullName = "warnshadow", description="show assembler warnings about symbol shadowing") val sourceDirs by cli.option(ArgType.String, fullName="srcdirs", description = "list of extra paths, separated with ${File.pathSeparator}, to search in for imported modules").multiple().delimiter(File.pathSeparator) val includeSourcelines by cli.option(ArgType.Boolean, fullName = "sourcelines", description = "include original Prog8 source lines in generated asm code") val splitWordArrays by cli.option(ArgType.Boolean, fullName = "splitarrays", description = "treat all word arrays as tagged with @split to make them lsb/msb split in memory") @@ -113,6 +114,7 @@ private fun compileMain(args: Array): Boolean { filepath, dontOptimize != true, dontWriteAssembly != true, + warnSymbolShadowing == true, quietAssembler == true, asmListfile == true, includeSourcelines == true, @@ -180,6 +182,7 @@ private fun compileMain(args: Array): Boolean { filepath, dontOptimize != true, dontWriteAssembly != true, + warnSymbolShadowing == true, quietAssembler == true, asmListfile == true, includeSourcelines == true, diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 3994b3090..974a83875 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -30,6 +30,7 @@ class CompilationResult(val compilerAst: Program, // deprecated, use codegenAs class CompilerArguments(val filepath: Path, val optimize: Boolean, val writeAssembly: Boolean, + val warnSymbolShadowing: Boolean, val quietAssembler: Boolean, val asmListfile: Boolean, val includeSourcelines: Boolean, @@ -67,6 +68,7 @@ fun compileProgram(args: CompilerArguments): CompilationResult? { compilationOptions = options with(compilationOptions) { + warnSymbolShadowing = args.warnSymbolShadowing optimize = args.optimize asmQuiet = args.quietAssembler asmListfile = args.asmListfile diff --git a/compiler/test/TestCompilerOnExamples.kt b/compiler/test/TestCompilerOnExamples.kt index 60b15c4bb..7332ef35a 100644 --- a/compiler/test/TestCompilerOnExamples.kt +++ b/compiler/test/TestCompilerOnExamples.kt @@ -27,6 +27,7 @@ private fun compileTheThing(filepath: Path, optimize: Boolean, target: ICompilat filepath, optimize, writeAssembly = true, + warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, includeSourcelines = false, diff --git a/compiler/test/TestCompilerOptionLibdirs.kt b/compiler/test/TestCompilerOptionLibdirs.kt index d38241b04..0a2ec7645 100644 --- a/compiler/test/TestCompilerOptionLibdirs.kt +++ b/compiler/test/TestCompilerOptionLibdirs.kt @@ -44,6 +44,7 @@ class TestCompilerOptionSourcedirs: FunSpec({ filepath = filePath, optimize = false, writeAssembly = true, + warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, includeSourcelines = false, diff --git a/compiler/test/helpers/compileXyz.kt b/compiler/test/helpers/compileXyz.kt index a01401bff..831e3cbba 100644 --- a/compiler/test/helpers/compileXyz.kt +++ b/compiler/test/helpers/compileXyz.kt @@ -24,6 +24,7 @@ internal fun compileFile( filepath, optimize, writeAssembly = writeAssembly, + warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, includeSourcelines = false, diff --git a/docs/source/compiling.rst b/docs/source/compiling.rst index aab1cae44..522b88000 100644 --- a/docs/source/compiling.rst +++ b/docs/source/compiling.rst @@ -151,6 +151,15 @@ One or more .p8 module files Note that it is possible to use the watch mode with multiple modules as well, but it will recompile everything in that list even if only one of the files got updated. +``-warnshadow`` + Tells the assembler to issue warning messages about symbol shadowing. + These *can* be problematic, but usually aren't because prog8 has different scoping rules + than the assembler has. + You may want to watch out for shadowing of builtin names though. Especially 'a', 'x' and 'y' + as those are the cpu register names and if you shadow those, the assembler might + interpret certain instructions differently and produce unexpected opcodes (like LDA X getting + turned into TXA, or not, depending on the symbol 'x' being defined in your own assembly code or not) + ``-quietasm`` Don't print assembler output results. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index b40e23934..f0f8cc29f 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,10 +1,12 @@ TODO ==== +- [on branch: shadowing-fixes] add shadowing warning to asm and fix shadowing errors +- prefix prog8 subroutines with p8s_ instead of p8_ to not let them clash with variables in the asm + - allow 'chained' array indexing for expressions: value = ptrarray[0][0] - allow 'chained' array indexing for assign targets: ptrarray[0][0] = 42 this is just evaluating the lhs as a uword pointer expression - [on branch: shortcircuit] investigate McCarthy evaluation again? this may also reduce code size perhaps for things like if a>4 or a<2 .... -- [on branch: shadowing-fixes] add shadowing warning to asm and fix shadowing errors - IR: reduce the number of branch instructions such as BEQ, BEQR, etc (gradually), replace with CMP(I) + status branch instruction - IR: reduce amount of CMP/CMPI after instructions that set the status bits correctly (LOADs? INC? etc), but only after setting the status bits is verified! diff --git a/examples/test.p8 b/examples/test.p8 index f3d9c10ee..438066b17 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,11 +1,13 @@ %import textio -%import string -%import floats %zeropage basicsafe main { sub start() { - float fl = floats.parse_f("-123.45678e20") - floats.print_f(fl) + thing() + } + sub thing() { + ubyte start=22 + txt.print_ub(start) + txt.print_uw(&main.start) } } diff --git a/httpCompilerService/src/prog8/http/TestHttp.kt b/httpCompilerService/src/prog8/http/TestHttp.kt index 8e43851ea..cbb0a101b 100644 --- a/httpCompilerService/src/prog8/http/TestHttp.kt +++ b/httpCompilerService/src/prog8/http/TestHttp.kt @@ -34,6 +34,7 @@ class RequestParser : Take { Path(a), optimize = true, writeAssembly = true, + warnSymbolShadowing = false, compilationTarget = "c64", symbolDefs = emptyMap(), quietAssembler = false,