diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000..1c921b207 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,45 @@ +# EditorConfig for Prog8 projects +# Ensures consistent formatting across editors and team members +# See https://editorconfig.org + +root = true + +# All files +[*] +charset = utf-8 +end_of_line = lf +insert_final_newline = true +trim_trailing_whitespace = true + +# Prog8 source files (.p8) +[*.p8] +indent_style = space +indent_size = 4 + +# Assembly files (.asm) - 64tass syntax +[*.asm] +indent_style = space +indent_size = 4 + +# Kotlin compiler source files +[*.kt] +indent_style = space +indent_size = 4 +trim_trailing_whitespace = false + +# Documentation files +[*.md] +trim_trailing_whitespace = false + +[*.rst] +trim_trailing_whitespace = false + +# Gradle build files +[*.gradle.kts] +indent_style = space +indent_size = 4 + +# Git ignore files +[.gitignore] +indent_style = space +indent_size = 2 diff --git a/.qwen/settings.json b/.qwen/settings.json index ef439cd0e..f54582710 100644 --- a/.qwen/settings.json +++ b/.qwen/settings.json @@ -31,7 +31,8 @@ "Bash(xargs *)", "Bash(wc *)", "Bash(do *)", - "Bash(done)" + "Bash(done)", + "Bash(diff *)" ] } } \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 5bffea81a..28c68c670 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,38 @@ When debugging compiler issues, **FIRST try compiling with the `-noopt` switch** - **Problem gone with `-noopt`**: Issue is in **optimization phases** (`optimizeAst()`, `UnusedCodeRemover`, `Inliner`, etc.) - **Problem persists with `-noopt`**: Issue is in **parsing, semantic analysis, symbol table, or code generation** +## DEBUGGING TIP: Use `-compareir` to see what changed +When investigating optimization-related issues or tracking regressions: +```bash +# Compile without optimizations (baseline) +prog8c -target virtual -noopt -out dir program.p8 + +# Compile with optimizations and compare +prog8c -target virtual -compareir dir/program_noopt.p8ir program.p8 +``` +**Output shows:** +- Instruction/chunk/register count changes with percentages +- First 10 actual instruction differences +- Helps identify which optimization transformed the code + +## DEBUGGING TIP: Use `-vmtrace` to trace execution +When debugging VM execution or control flow issues: +```bash +prog8c -target virtual -vm program.p8ir -vmtrace +prog8c -target virtual -emu -vmtrace program.p8 +``` +**Output format:** `[chunkName:instructionIndex] instruction` +- Shows each executed IR instruction with location +- Useful for understanding control flow and finding where execution diverges +- **Only works on virtual target** + +## Typical debugging workflow: +1. **Quick check:** `-check` for syntax errors +2. **Isolate:** `-noopt` to determine if problem is optimizer-related +3. **Compare:** `-compareir` to see what instructions changed +4. **Trace:** `-vmtrace` to watch actual execution flow +5. **Deep dive:** `-printast1` / `-printast2` for compiler internals + ## IMPORTANT: SymbolTable cachedFlat cache The `SymbolTable` class has a **cached `flat` property** for fast symbol lookups. @@ -52,9 +84,9 @@ For problems that **ONLY occur with the 'virtual' target**, **ONLY modify these **DO NOT modify** `compilerAst`, `simpleAst`, `codeGenCpu6502`, etc. The IR codegen has its own separate handling for symbol tables, AST transformations, and unused code removal. ## CRITICAL: NO FORMATTING -- NEVER run formatters (black, ruff, prettier, etc.) after edits -- Preserve my exact indentation, line lengths, and spacing -- Make ONLY the requested changes, nothing else +- DO NOT change indentation and formatting of lines that are not being modified. NEVER run formatters (black, ruff, prettier, etc.) after edits, +- .editorconfig handles basic formatting (indentation, line endings, whitespace) +- Make ONLY the requested changes, touch nothing else ## Prog8 language feature hints @@ -92,8 +124,14 @@ For problems that **ONLY occur with the 'virtual' target**, **ONLY modify these - the CPU hardware stack can be manipulated via builtin functions: push(), pushw(), pushl(), pushf() and pop(), popw(), popl(), popf(). These can manually implement recursion if needed. ### Logic & Control Flow -- boolean operators: 'and', 'or', 'xor', 'not'. Bitwise: '&', '|', '^', '~', '<<', '>>'. See docs/source/programming.rst. -- **Short-circuit evaluation**: Logical `and` and `or` use short-circuit evaluation! In `a and b`, if `a` is false, `b` is NOT evaluated. In `a or b`, if `a` is true, `b` is NOT evaluated. This is important when `b` has side effects or could cause errors. +- **Logical operators** (short-circuit, boolean only): `and`, `or`, `xor`, `not` + - Use in conditions: `if x == 0 or y > 10 { ... }` + - Short-circuit: In `a and b`, if `a` is false, `b` is NOT evaluated. In `a or b`, if `a` is true, `b` is NOT evaluated. + - This is important when `b` has side effects or could cause errors. +- **Bitwise operators** (work on integer bits): `&`, `|`, `^`, `~`, `<<`, `>>` + - Use for bit manipulation: `mask = mask | FLAG_ACTIVE` + - Test bits: `if (mask & FLAG) != 0 { ... }` + - **Common mistake**: Don't use `and`/`or` for bitmask operations - use `&`/`|` instead! - CPU status flags: if_cs, if_cc, if_z, if_nz, etc. compile to single 6502 branch instructions. - use 'when' statements with choice blocks instead of multiple 'if' statements. - use 'repeat' instead of loops when iteration count is not needed. @@ -144,10 +182,96 @@ For problems that **ONLY occur with the 'virtual' target**, **ONLY modify these ### Syntax & Formatting - **numeric literal syntax**: `$` prefix for hex (`$FF` not `0xFF`), `%` prefix for binary (`%1010` not `0b1010`). Underscores allowed for readability: `25_000_000`. No leading-zero octal notation. **No type suffixes**: long literals are just regular numbers (e.g., `12345678` not `12345678L`), the type is determined by context (variable type or cast). -- **for loop syntax**: `for i in 0 to 10 { ... }` (use downto when counting down) - NOT `for i = 0 to 10` or C-style `for(i=0; i<10; i++)` +- **for loop syntax**: `for i in 0 to 10 { ... }` (use downto when counting down) - NOT `for i = 0 to 10` or C-style `for(i=0; i<10; i++)`. Loop variables must be declared separately before the loop - inline declaration like `for ubyte i in 0 to 10` is not supported. - **semicolons start comments**: `; this is a comment` - they do NOT end statements. There is NO statement separator (unlike C/Java's `;`). One statement per line only. Multi-line comments use `/* ... */`. -- Prog8 source files are indented with 4 spaces, no tabs. Assembly source files (*.asm) can use spaces or tabs. +- **Indentation**: See `.editorconfig` (4 spaces for .p8 and .asm files, no tabs). - **The assembly source code uses 64tass syntax, NOT ca65/cc65 or other assemblers.** Key 64tass syntax: `.proc`/`.pend` for procedures, `_label` for local labels, `.byte`/`.word`/`.dword` for data, `= ` for equates, zero-page variables defined with `=`. **Instructions like `rol`, `ror`, `asl`, `lsr` require an explicit operand** - use `rol a`, `ror a`, etc. for the accumulator, not just `rol` or `ror`. +- **Character encoding**: 6502 targets use **PETSCII** by default. The `virtual` target uses **ISO** encoding. + - **6502 targets**: Use `txt.lowercase()` at program start for lowercase display. + - **Virtual target**: Use `%encoding iso` directive and call `txt.iso()` at program start. + - **Exception for x16emu debugging**: When running with `x16emu -echo iso` to see console output, use ISO encoding even on 6502 targets so the echoed text is readable. +- **String arrays**: Use `str[N]` for arrays of strings (e.g., `str[12] types = ["sword", "axe", ...]`). Access with `types[i]`. +- **String buffer pre-allocation**: **CRITICAL** - Empty strings don't allocate space! Always pre-allocate: + - WRONG: `str buffer = ""` (no space allocated, `strings.append()` will fail) + - RIGHT: `str buffer = "." * 50` (allocates 50 characters) +- **String concatenation pattern**: Use `strings` module functions with pre-allocated buffers: + ```prog8 + %import strings + str buffer = "." * 50 ; pre-allocate + void strings.copy(source, buffer) ; initialize + void strings.append(buffer, "text") ; concatenate + ``` + **WARNING**: String concatenation is EXPENSIVE on 6502! Only use when you truly need a combined string. If fragments can be handled separately (e.g., printing multiple strings in sequence), do NOT concatenate - just process each fragment separately: + ```prog8 + ; EXPENSIVE - don't do this unless necessary + void strings.append(buffer, prefix) + void strings.append(buffer, suffix) + txt.print(buffer) + + ; CHEAP - prefer this when possible + txt.print(prefix) + txt.print(suffix) + ``` +- **Use `len()` for array sizes**: Don't hardcode array lengths - use `len(array)`: + ```prog8 + ; WRONG: magic number + ubyte i = math.rnd() % 20 + + ; RIGHT: self-documenting + ubyte i = math.rnd() % len(weapons.prefixes) + ``` +- **Enums**: Prog8 enums are declared inside a block and use `Enum::Value` syntax (double colon, not dot): + ```prog8 + enum Priority { + LOW = 1, + NORMAL, ; auto-numbered to 2 + HIGH, ; auto-numbered to 3 + EXTREME = 255 + } + + ; Generates these constants: + const ubyte Priority::LOW = 1 + const ubyte Priority::NORMAL = 2 + const ubyte Priority::HIGH = 3 + const ubyte Priority::EXTREME = 255 + + ; Usage: + ubyte p = Priority::HIGH ; NOT Priority.HIGH + ``` + **When to use enums vs const**: Use enums for sets of related named values (states, types, classes). Use `const` for standalone values: + ```prog8 + ; GOOD: enum for related choices + enum CharClass { + FIGHTER = 1, + MAGE = 2, + CLERIC = 3 + } + + ; GOOD: const for standalone values + const ubyte MIN_HP = 1 + const ubyte BASE_AC = 10 + ``` + **Note**: Enum values are accessed directly by name within their block. For values needed across multiple blocks, use `const` in a shared block instead. +- **Avoid `globals.XXXX` code smell**: If you find yourself prefixing many constants with `globals.`, consider: + 1. Moving constants closer to where they're used (inside the relevant block) + 2. Using literals directly for obvious values (like `3` for 3d6) +- **Array size inference**: When declaring an array with an initializer list, you don't need to specify the size - Prog8 infers it automatically: + ```prog8 + ; Explicit size (works but verbose) + ubyte[12] types = ["sword", "axe", "bow", ...] + + ; Inferred size (cleaner, preferred) + str[] types = ["sword", "axe", "bow", "staff", "mace", "dagger"] + ``` + This is especially useful for string arrays and lookup tables. Use `len(array)` to get the size when needed. +- **Use strings library for character operations**: The `strings` module has functions for character manipulation and checks (case conversion, character class tests, etc.). Use these instead of manual ASCII/PETSCII arithmetic. +- **String functions return useful lengths**: Several `strings` library routines return the length of the string they operated on. This return value is often voided, but capturing it avoids redundant `len()` calls: + ```prog8 + ; returning length, Useful for: repeated appends, tracking buffer usage, avoiding redundant len() calls + len = strings.copy(dest, src) ; returns copied length + len = strings.append(buf, text) ; returns resulting length + len = strings.upper(mystr) ; returns string length + ``` ## Other Key differences from other languages (C, Python, etc.) - **type casting syntax**: Use `expression as ` to cast (e.g., `bytevar as word`, `(a+b) as uword`). This is required for type conversions. @@ -192,7 +316,7 @@ For problems that **ONLY occur with the 'virtual' target**, **ONLY modify these ### When you MUST rebuild AND reinstall the compiler **After ANY change to:** -1. **Kotlin compiler source code** (`.kt` files in any module) +1. **Kotlin compiler source code** (`.kt` files in any module, or the `.g4` grammar file) 2. **Standard library Prog8 files** (`compiler/res/prog8lib/**/*.p8`) 3. **Standard library assembly files** (`compiler/res/prog8lib/**/*.asm`) diff --git a/codeCore/src/prog8/code/target/VMTarget.kt b/codeCore/src/prog8/code/target/VMTarget.kt index 741ac8262..e86b9428d 100644 --- a/codeCore/src/prog8/code/target/VMTarget.kt +++ b/codeCore/src/prog8/code/target/VMTarget.kt @@ -69,6 +69,10 @@ class VMTarget: ICompilationTarget, } override fun launchEmulator(selectedEmulator: Int, programNameWithPath: Path, quiet: Boolean) { + launchEmulatorWithTrace(programNameWithPath, quiet, traceEnabled = false) + } + + fun launchEmulatorWithTrace(programNameWithPath: Path, quiet: Boolean, traceEnabled: Boolean) { if(!quiet) println("\nStarting Virtual Machine...") @@ -76,7 +80,7 @@ class VMTarget: ICompilationTarget, val vm = Class.forName("prog8.vm.VmRunner").getDeclaredConstructor().newInstance() as IVirtualMachineRunner val withExt = if(programNameWithPath.extension=="p8ir") programNameWithPath else programNameWithPath.resolveSibling("${programNameWithPath.name}.p8ir") if(withExt.isReadable()) - vm.runProgram(withExt.readText(), quiet) + vm.runProgram(withExt.readText(), quiet, traceEnabled) else throw java.nio.file.NoSuchFileException(withExt.name, null, "not a .p8ir file") } @@ -92,7 +96,7 @@ class VMTarget: ICompilationTarget, interface IVirtualMachineRunner { - fun runProgram(irSource: String, quiet: Boolean) + fun runProgram(irSource: String, quiet: Boolean, traceEnabled: Boolean = false) } private class VirtualZeropage(options: CompilationOptions): Zeropage(options) { diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt index 98e8190be..1e67a4e43 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt @@ -1555,7 +1555,7 @@ class IRCodeGen( addToResult(result, leftTr, leftTr.resultReg, -1) if (number!=null) { val elseBranch: Opcode - var useCmpi = false // for the branch opcodes that have been converted to CMPI + BSTxx form already + var useCmpi: Boolean // for the branch opcodes that have been converted to CMPI + BSTxx form already // Optimization: when comparing signed integer with 0, we can use direct status flag branches // instead of CMPI #0 + branch, because the previous LOAD instruction already sets the N and Z flags. @@ -2186,7 +2186,7 @@ class IRCodeGen( } // store with field offset - var valueRegister = existingValueRegister + val valueRegister = existingValueRegister val irdt = irType(type) if(valueIsZero && valueRegister<0) { addInstr(result, IRInstruction(Opcode.STOREZI, irdt, reg1 = addressReg, immediate = offset.toInt()), null) diff --git a/compiler/build.gradle.kts b/compiler/build.gradle.kts index 961afffdf..6d1496a75 100644 --- a/compiler/build.gradle.kts +++ b/compiler/build.gradle.kts @@ -13,6 +13,7 @@ dependencies { implementation(project(":codeGenCpu6502")) implementation(project(":codeGenIntermediate")) implementation(project(":codeGenExperimental")) + implementation(project(":intermediate")) implementation(project(":virtualmachine")) implementation("org.jetbrains.kotlinx:kotlinx-cli:0.3.6") implementation("com.michael-bull.kotlin-result:kotlin-result-jvm:2.3.1") diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 8812263bf..b98e975a4 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -12,6 +12,8 @@ import prog8.compiler.CompilationResult import prog8.compiler.CompilerArguments import prog8.compiler.ErrorReporter import prog8.compiler.compileProgram +import prog8.intermediate.IRFileReader +import prog8.intermediate.Opcode import java.io.File import java.net.URI import java.nio.file.* @@ -70,10 +72,12 @@ private fun compileMain(args: Array): Boolean { val varsGolden by cli.option(ArgType.Boolean, fullName = "varsgolden", description = "put uninitialized variables in 'golden ram' memory area instead of at the end of the program. On the cx16 target this is $0400-07ff. This is unavailable on other systems.") val varsHighBank by cli.option(ArgType.Int, fullName = "varshigh", description = "put uninitialized variables in high memory area instead of at the end of the program. On the cx16 target the value specifies the HIRAM bank to use, on other systems this value is ignored.") val startVm by cli.option(ArgType.Boolean, fullName = "vm", description = "run a .p8ir IR source file in the embedded VM") + val vmTrace by cli.option(ArgType.Boolean, fullName = "vmtrace", description = "trace VM execution instruction by instruction (use with -vm or -emu)") val warnSymbolShadowing by cli.option(ArgType.Boolean, fullName = "warnshadow", description="show assembler warnings about symbol shadowing") val warnImplicitTypeCasts by cli.option(ArgType.Boolean, fullName = "warnimplicitcasts", description="show compiler warnings about implicit casts from a smaller to a larger type") val watchMode by cli.option(ArgType.Boolean, fullName = "watch", description = "continuous compilation mode (watch for file changes)") val version by cli.option(ArgType.Boolean, fullName = "version", description = "print compiler version and exit") + val compareIR by cli.option(ArgType.String, fullName = "compareir", description = "compare generated .p8ir file with existing .p8ir file") val moduleFiles by cli.argument(ArgType.String, fullName = "modules", description = "main module file(s) to compile").optional().multiple(999) try { @@ -176,7 +180,7 @@ private fun compileMain(args: Array): Boolean { } if(startVm==true) { - runVm(moduleFiles.first(), quietAll==true) + runVm(moduleFiles.first(), quietAll==true, vmTrace==true) return true } @@ -332,9 +336,18 @@ private fun compileMain(args: Array): Boolean { val programNameInPath = outputPath.resolve(compilationResult.compilerAst.name) - if (startEmulator1 == true) - compilationResult.compilationOptions.compTarget.launchEmulator(1, programNameInPath, quietAll==true) - else if (startEmulator2 == true) + if (compareIR != null) { + compareIrFiles(outputPath.resolve("${compilationResult.compilerAst.name}.p8ir"), Path(compareIR!!)) + } + + if (startEmulator1 == true) { + if (compilationResult.compilationOptions.compTarget is VMTarget) { + (compilationResult.compilationOptions.compTarget as VMTarget).launchEmulatorWithTrace( + programNameInPath, quietAll==true, vmTrace==true) + } else { + compilationResult.compilationOptions.compTarget.launchEmulator(1, programNameInPath, quietAll==true) + } + } else if (startEmulator2 == true) compilationResult.compilationOptions.compTarget.launchEmulator(2, programNameInPath, quietAll==true) } } @@ -380,10 +393,149 @@ private fun processSymbolDefs(symbolDefs: List): Map? { return result } -fun runVm(irFilename: String, quiet: Boolean) { +fun runVm(irFilename: String, quiet: Boolean, traceEnabled: Boolean = false) { val irFile = Path(irFilename) val vmdef = VMTarget() - vmdef.launchEmulator(0, irFile, quiet) + vmdef.launchEmulatorWithTrace(irFile, quiet, traceEnabled) +} + +private fun compareIrFiles(newFile: Path, baselineFile: Path) { + if (!baselineFile.toFile().exists()) { + System.err.println("Compare error: baseline file not found: $baselineFile") + return + } + if (!newFile.toFile().exists()) { + System.err.println("Compare error: generated file not found: $newFile") + return + } + + // Parse both IR files properly using IRFileReader + val baselineProgram = IRFileReader().read(baselineFile) + val newProgram = IRFileReader().read(newFile) + + // Extract metrics from parsed IR programs + val baselineMetrics = extractIrMetrics(baselineProgram) + val newMetrics = extractIrMetrics(newProgram) + + println("\nIR File Comparison: ${newFile.fileName} vs ${baselineFile.fileName}") + println("=".repeat(70)) + + println("\nSummary:") + println(" Instructions: ${formatMetric(newMetrics.instructions, baselineMetrics.instructions)}") + println(" Chunks: ${formatMetric(newMetrics.chunks, baselineMetrics.chunks)}") + println(" Registers: ${formatMetric(newMetrics.registers, baselineMetrics.registers)}") + println(" File size: ${formatMetric(newFile.toFile().length(), baselineFile.toFile().length())}") + + // Compare instruction sequences + val baselineCode = extractCodeInstructions(baselineProgram) + val newCode = extractCodeInstructions(newProgram) + + if (baselineCode == newCode) { + println("\n✓ No code differences (generated IR instructions are identical).") + } else { + val differences = findCodeDifferences(baselineCode, newCode) + println("\n✗ Found ${differences.size} instruction difference(s) in code chunks.") + + // Show first few differences + if (differences.isNotEmpty()) { + println("\nFirst differences:") + differences.take(10).forEach { (index, baselineIns, newIns) -> + println(" [$index] $baselineIns → $newIns") + } + if (differences.size > 10) { + println(" ... and ${differences.size - 10} more differences") + } + } + println("\n (Use 'diff' command on the .p8ir files for detailed comparison)") + } + println() +} + +private fun formatMetric(newVal: Int, baselineVal: Int): String { + val delta = newVal - baselineVal + val pct = if (baselineVal != 0) (delta * 100.0 / baselineVal) else 0.0 + val deltaStr = if (delta == 0) "(no change)" else if (delta > 0) "(+${delta}, +${pct.toInt()}%)" else "(${delta}, ${pct.toInt()}%)" + return "$newVal (new) vs $baselineVal (baseline) $deltaStr" +} + +private fun formatMetric(newVal: Long, baselineVal: Long): String { + val delta = newVal - baselineVal + val pct = if (baselineVal != 0L) (delta * 100.0 / baselineVal) else 0.0 + val deltaStr = if (delta == 0L) "(no change)" else if (delta > 0L) "(+${delta}, +${pct.toInt()}%)" else "(${delta}, ${pct.toInt()}%)" + return "$newVal bytes (new) vs $baselineVal bytes (baseline) $deltaStr" +} + +data class InstructionDiff(val index: Int, val baseline: String, val new: String) + +private fun findCodeDifferences(baseline: List, new: List): List { + val diffs = mutableListOf() + val maxLines = maxOf(baseline.size, new.size) + for (i in 0 until maxLines) { + val newLine = if (i < new.size) new[i] else "" + val baselineLine = if (i < baseline.size) baseline[i] else "" + if (newLine != baselineLine) { + diffs.add(InstructionDiff(i, baselineLine, newLine)) + } + } + return diffs +} + +data class IrMetrics(val instructions: Int, val chunks: Int, val registers: Int) + +private fun extractIrMetrics(program: prog8.intermediate.IRProgram): IrMetrics { + var instructions = 0 + var chunks = 0 + val readRegs = mutableSetOf() + val writeRegs = mutableSetOf() + + // Count chunks and instructions from all subroutines + program.allSubs().forEach { sub -> + sub.chunks.forEach { chunk -> + chunks++ + instructions += chunk.instructions.size + + // Extract register usage from each chunk + chunk.instructions.forEach { ins -> + ins.reg1?.let { readRegs.add(it) } + ins.reg2?.let { readRegs.add(it) } + ins.reg3?.let { + if (ins.opcode == Opcode.CONCAT || ins.opcode == Opcode.EXT || + ins.opcode == Opcode.EXTS) { + readRegs.add(it) + } else { + writeRegs.add(it) + } + } + ins.fpReg1?.let { readRegs.add(it) } + ins.fpReg2?.let { readRegs.add(it) } + } + } + } + + // Also count global init chunks + program.globalInits.instructions.forEach { ins -> + ins.reg1?.let { readRegs.add(it) } + ins.reg2?.let { readRegs.add(it) } + ins.reg3?.let { writeRegs.add(it) } + } + + val registers = readRegs.size + writeRegs.size + return IrMetrics(instructions, chunks, registers) +} + +private fun extractCodeInstructions(program: prog8.intermediate.IRProgram): List { + val code = mutableListOf() + + // Get all instructions from all subroutines + program.allSubs().forEach { sub -> + sub.chunks.forEach { chunk -> + chunk.instructions.forEach { ins -> + code.add(ins.toString()) + } + } + } + + return code } diff --git a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt index 2c14d455b..07bd4a3be 100644 --- a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt +++ b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt @@ -1506,7 +1506,10 @@ data class IdentifierReference(val nameInSource: List, override val posi val vardecl = definingScope.lookup(nameInSource.take(1)) as? VarDecl if (vardecl?.datatype?.isPointer != true) return DataType.UNDEFINED - require(vardecl.datatype.subType!=null) { "pointer type should point to a struct ${vardecl.position}" } + if(vardecl.datatype.subType==null) { + // Pointer to primitive type - return the pointer type itself + return vardecl.datatype + } struct = vardecl.datatype.subType!! fieldDt = vardecl.datatype } @@ -1519,7 +1522,8 @@ data class IdentifierReference(val nameInSource: List, override val posi // was last path element return fieldDt } - struct = fieldDt.subType ?: return DataType.UNDEFINED + // If field is pointer to primitive (like str), we can't deref further + struct = fieldDt.subType ?: return fieldDt } return fieldDt ?: DataType.UNDEFINED } diff --git a/docs/source/compiling.rst b/docs/source/compiling.rst index 81e6aeba5..511030149 100644 --- a/docs/source/compiling.rst +++ b/docs/source/compiling.rst @@ -173,6 +173,12 @@ One or more .p8 module files ``-check`` Quickly check the program for errors. No actual compilation will be performed. +``-compareir `` + Compare the generated IR file with a baseline IR file. + Shows a summary of differences including instruction count, chunk count, register usage, and file size. + Also displays the first 10 instruction differences to help identify what changed. + Useful for comparing optimized vs non-optimized builds, or tracking changes between compiler versions. + ``-D SYMBOLNAME=VALUE`` Add this user-defined symbol directly to the beginning of the generated assembly file. Can be repeated to define multiple symbols. @@ -298,6 +304,11 @@ One or more .p8 module files ``-vm`` load and run a 'p8ir' intermediate representation file in the internal VirtualMachine instead of compiling a prog8 program file. +``-vmtrace`` + Enable instruction-by-instruction tracing when running the virtual machine (use with ``-vm`` or ``-emu`` on the virtual target). + Prints each executed IR instruction with its location (chunk name and instruction index). + Useful for debugging program behavior and understanding control flow in the IR code. + ``-warnimplicitcasts`` Give warnings for lines where a silent (implicit) type cast is done from a smaller to a larger type. Example: when a byte is assigned to a word variable. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 73dd836f7..099145b57 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -109,6 +109,7 @@ Libraries Optimizations ------------- +- for loops where the counter variable is unused in the loop body and count is constant -> simple repeat loops - optimize the endless sequences of lda/sta's to juggle round longs for example when calling prog8_math.multiply_longs -> shorter to use a loop? - bind types in the Ast much sooner than the simplifiedAst creation, so that we maybe could get rid of InferredType ? - Port more benchmarks from https://thred.github.io/c-bench-64/ to prog8 and see how it stacks up. (see benchmark-c/ directory) diff --git a/grammar-changes.md b/grammar-changes.md index 32f88988a..cc9a334f6 100644 --- a/grammar-changes.md +++ b/grammar-changes.md @@ -4,6 +4,252 @@ This document outlines recommended improvements to the `Prog8ANTLR.g4` grammar file, focusing on clearer parse errors, rule optimization, and maintainability. +--- + +# Language Design Inconsistencies + +These are **not** ANTLR implementation issues - they are **language design inconsistencies** in the grammar itself that would confuse users and make the language harder to learn. + +## 1. Inconsistent Statement Terminators + +**Current Grammar:** +```antlr +statement_block : '{' EOL? (statement | EOL) * '}'; +module: EOL* (module_element (EOL+ module_element)*)? EOL* EOF; +``` + +**Problem:** +- Module elements **require** `EOL+` between them +- Statements inside blocks **do not require** EOL separation + +This means: +```prog8 +{ a=1 b=2 } ; valid - no EOL between statements +{ a=1 + b=2 } ; valid - with EOL +``` + +But at module level: +```prog8 +block1 { } block2 { } ; INVALID - needs EOL between blocks +``` + +**Recommendation:** Make EOL requirements consistent - either require EOL everywhere or make it optional everywhere. + +--- + +## 2. `enum` and `alias` as Statements (Semantically Wrong) + +**Current Grammar:** +```antlr +block_statement: enum | alias; +statement: enum | alias; +``` + +**Problem:** Enums and aliases can be defined inside subroutine bodies: +```prog8 +myblock { + enum Status { OK, FAIL } ; block level - makes sense + + sub foo() { + enum Status2 { A, B } ; inside subroutine - weird! + alias x = y ; also weird inside subroutine + } +} +``` + +**Recommendation:** Remove `enum` and `alias` from the `statement` rule. They should only be allowed in `block_statement`. + +--- + +## 3. Inconsistent Optional EOL Before Braces + +**Current Grammar:** +```antlr +block: identifier integerliteral? EOL? '{' EOL? ... +statement_block : '{' EOL? ... +enum: ENUM identifier '{' EOL? ... +structdeclaration: STRUCT identifier '{' EOL? ... +``` + +**Problem:** Some constructs allow optional EOL before `{`, but there's no clear pattern. Why is it `EOL?` everywhere instead of consistent? + +**Recommendation:** Either require EOL before `{` everywhere for consistency, or document why it varies. + +--- + +## 4. `VOID` Used in Multiple Incompatible Contexts + +**Current Grammar:** +```antlr +assign_target: VOID #VoidTarget; +functioncall_stmt: VOID? scoped_identifier '(' ...; +``` + +**Problem:** `VOID` is used as: +1. An assignment target (to discard return values): `void, x = func()` +2. A statement prefix (to discard return values): `void func()` + +These are different use cases but use the same keyword. The grammar allows: +```prog8 +void = func() ; ??? Is this valid? What does this mean? +``` + +**Recommendation:** Consider different syntax for discarding return values in different contexts, or clarify the semantics. + +--- + +## 5. `repeat` Loop Optional Expression - Unclear Semantics + +**Current Grammar:** +```antlr +repeatloop: 'repeat' expression? EOL? (statement | statement_block); +unrollloop: 'unroll' expression EOL? (statement | statement_block); +``` + +**Problem:** +- `repeat` has optional `expression?` - what does it mean when omitted? +- `unroll` **requires** an expression +- Why the inconsistency? + +```prog8 +repeat 5 { ... } ; repeat 5 times? +repeat { ... } ; repeat forever? What's the difference from 'while true'? +unroll 5 { ... } ; must specify count +``` + +**Recommendation:** Either make the expression required for `repeat`, or document clearly what omission means. Make `repeat` and `unroll` consistent. + +--- + +## 6. Pointer Dereference Grammar is Admittedly "Cursed" + +**Current Grammar:** +```antlr +pointerdereference: (prefix = scoped_identifier '.')? derefchain ('.' field = identifier)?; +derefchain: singlederef ('.' singlederef)*; +singlederef: identifier arrayindex? POINTER; +``` + +**Comment in grammar:** +> "This is a cursed mix of IdentifierReference and binary expressions with '.' dereference operators." + +**Problems:** +1. The grammar allows `foo.bar^^.baz` but what about `foo^^.bar`? +2. Array indexing on pointers: `ptr[0]^^` vs `ptr^^[0]` - which is valid? +3. The comment admits this is a hack that needs rewriting + +**Recommendation:** This needs a proper redesign. Consider treating `^^` as a postfix operator with clear precedence rules. + +--- + +## 7. `sizeof` Allows Arbitrary Expressions + +**Current Grammar:** +```antlr +expression: 'sizeof' '(' sizeof_argument ')'; +sizeof_argument: basedatatype | expression | pointertype; +``` + +**Problem:** You can do `sizeof(5)` (expression) but what does that mean? Shouldn't it be `sizeof(byte)` or `sizeof(myvar)` only? + +```prog8 +sizeof(5) ; ??? Returns size of what type? +sizeof(3 + 4) ; ??? +sizeof(byte) ; Makes sense +sizeof(myvar) ; Makes sense +``` + +**Recommendation:** Restrict `sizeof_argument` to `basedatatype | scoped_identifier | pointertype` - remove arbitrary `expression`. + +--- + +## 8. Trailing Commas Inconsistency + +**Current Grammar:** +```antlr +enum: '{' EOL? enum_member? (',' EOL? enum_member)* ','? EOL? '}'; ; trailing comma allowed +arrayliteral: '[' EOL? expression? (',' EOL? expression)* ','? EOL? ']'; ; trailing comma allowed +``` + +**Problem:** Some constructs allow trailing commas, others don't: +- ✅ `enum` - trailing comma allowed +- ✅ `arrayliteral` - trailing comma allowed +- ❌ Function parameters - no trailing comma shown +- ❌ Expression lists - no trailing comma shown +- ❌ Multi-assign targets - no trailing comma shown + +**Recommendation:** Make trailing commas consistently allowed (or consistently disallowed) across all list constructs. + +--- + +## 9. `const` Declaration - Optional Type is Confusing + +**Current Grammar:** +```antlr +constdecl: 'const' datatype? identifierlist '=' expression; +``` + +**Problem:** `datatype?` is optional, but then how is this different from a regular variable declaration? What happens if you omit the type? + +```prog8 +const x = 5 ; What type is x? Inferred? +const byte y = 5 ; Explicit type +``` + +**Recommendation:** Either make `datatype` required, or document clearly how type inference works for constants. + +--- + +## 10. `inline` Keyword Only for `asmsub` + +**Current Grammar:** +```antlr +asmsubroutine: INLINE? 'asmsub' asmsub_decl ... +``` + +**Problem:** `inline` can only prefix `asmsub`, not regular `sub`. But the documentation mentions inline functions - are they different from inline assembly? + +```prog8 +inline asmsub foo() { ... } ; Valid +inline sub bar() { ... } ; Invalid - syntax error +``` + +**Recommendation:** Either allow `inline` on regular subroutines too, or rename to `inlineasm` to clarify it's assembly-specific. + +--- + +## Summary of Language Design Issues + +**Note on evaluation criteria:** Issues are evaluated with Prog8's **retro 6502/BASIC target** in mind. Features that would be "archaic" in modern languages (like `ON...GOTO`) are **intentional and appropriate** for this platform. Issues listed here are about **internal consistency** and **clarity**, not about being "modern" vs "retro". + +| Issue | Severity | Recommendation | +|-------|----------|----------------| +| `enum`/`alias` as statements | High | Remove from `statement` rule | +| Pointer dereference grammar | High | Proper redesign needed | +| `repeat` optional expression | Medium | Clarify semantics or require expression | +| `sizeof` allows expressions | Medium | Restrict to types and identifiers | +| Inconsistent EOL requirements | Medium | Make consistent | +| Trailing commas inconsistency | Low | Make consistent | +| `const` optional type | Low | Require or document inference | +| `inline` only for `asmsub` | Low | Extend or rename | +| `VOID` in multiple contexts | Low | Clarify or differentiate | + +**Removed from list (retro-appropriate features):** +- `ON...GOTO` with `else` clause - **NOT a flaw**. Classic BASIC syntax appropriate for 6502 target. The `else` clause provides useful error handling for out-of-range indices (e.g., menu dispatch). Compiles efficiently to 6502 jump tables. See comment in `.g4` file. + +**Issues that may also be retro-appropriate (needs further review):** +- **Inconsistent EOL requirements** - May be intentional flexibility for different contexts (module vs block level) +- **`VOID` in multiple contexts** - May be intentional overloading for convenience (discard return value in different positions) +- **`inline` only for `asmsub`** - May be intentional distinction (inline assembly vs inline function expansion are different concepts) +- **Trailing commas inconsistency** - May reflect different use cases (enums/arrays are data, function calls are execution) + +These should be evaluated based on **what makes sense for 6502 programmers** coming from BASIC/assembly, not modern language conventions. + +--- + +# ANTLR4 Implementation Improvements + ## Current Issues Summary The Prog8 ANTLR4 grammar has several areas that could be improved: diff --git a/parser/src/main/antlr/Prog8ANTLR.g4 b/parser/src/main/antlr/Prog8ANTLR.g4 index 3006a03e9..bc5f48847 100644 --- a/parser/src/main/antlr/Prog8ANTLR.g4 +++ b/parser/src/main/antlr/Prog8ANTLR.g4 @@ -389,6 +389,10 @@ whenstmt: 'when' expression EOL? '{' EOL? (when_choice | EOL) * '}' EOL? ; when_choice: (expression_list | ELSE ) '->' (statement | statement_block ) ; +// ON...GOTO/ON...CALL with optional else clause - classic BASIC-style multi-way branch. +// This is intentional retro syntax appropriate for 6502/BASIC target audience. +// The else_part handles out-of-range indices (useful for menu dispatch with error handling). +// Compiles efficiently to 6502 jump tables. NOT a design flaw - fits the retro platform. ongoto: ON expression kind=(GOTO | CALL) directivenamelist EOL? else_part? ; staticstructinitializer: POINTER? scoped_identifier ':' arrayliteral ; diff --git a/virtualmachine/src/prog8/vm/VirtualMachine.kt b/virtualmachine/src/prog8/vm/VirtualMachine.kt index db45de086..102ad0b8e 100644 --- a/virtualmachine/src/prog8/vm/VirtualMachine.kt +++ b/virtualmachine/src/prog8/vm/VirtualMachine.kt @@ -51,6 +51,7 @@ class VirtualMachine(irProgram: IRProgram) { val callStack = ArrayDeque() val valueStack = ArrayDeque() // max VALUE_STACK_MAX entries var breakpointHandler: ((pcChunk: IRCodeChunk, pcIndex: Int) -> Unit)? = null // can set custom breakpoint handler + var traceEnabled: Boolean = false // enable instruction tracing var pcChunk = IRCodeChunk(null, null) var pcIndex = 0 var stepCount = 0 @@ -184,6 +185,10 @@ class VirtualMachine(irProgram: IRProgram) { } private fun dispatch(ins: IRInstruction) { + if (traceEnabled) { + val chunkLabel = pcChunk.label ?: "?" + println("[$chunkLabel:$pcIndex] $ins") + } when(ins.opcode) { Opcode.NOP -> nextPc() Opcode.LOAD -> InsLOAD(ins) @@ -3239,13 +3244,14 @@ internal fun ArrayDeque.popf(): Double { // probably called via reflection class VmRunner: IVirtualMachineRunner { - override fun runProgram(irSource: String, quiet: Boolean) { - runAndTestProgram(irSource, quiet) { /* no tests */ } + override fun runProgram(irSource: String, quiet: Boolean, traceEnabled: Boolean) { + runAndTestProgram(irSource, quiet, traceEnabled) { /* no tests */ } } - fun runAndTestProgram(irSource: String, quiet: Boolean = false, test: (VirtualMachine) -> Unit) { + fun runAndTestProgram(irSource: String, quiet: Boolean = false, traceEnabled: Boolean = false, test: (VirtualMachine) -> Unit = {}) { val irProgram = IRFileReader().read(irSource) val vm = VirtualMachine(irProgram) + vm.traceEnabled = traceEnabled // vm.breakpointHandler = { pcChunk, pcIndex -> // println("UNHANDLED BREAKPOINT") // println(" IN CHUNK: $pcChunk(${pcChunk.label}) INDEX: $pcIndex = INSTR ${pcChunk.instructions[pcIndex]}")