From 29254e892b56c50005c830074abc5c8ed2effff1 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 4 Oct 2026 00:40:19 +0200 Subject: [PATCH] m68k: no longer print recursion warnings --- AGENTS.md | 14 +++++++++- .../src/prog8/code/core/ICompilationTarget.kt | 7 +++++ .../src/prog8/code/target/Amiga1200Target.kt | 1 + .../src/prog8/code/target/Amiga500Target.kt | 1 + codeCore/src/prog8/code/target/C128Target.kt | 1 + codeCore/src/prog8/code/target/C64Target.kt | 1 + .../src/prog8/code/target/ConfigFileTarget.kt | 2 ++ codeCore/src/prog8/code/target/Cx16Target.kt | 1 + codeCore/src/prog8/code/target/PETTarget.kt | 1 + .../src/prog8/code/target/Qemu68kTarget.kt | 1 + codeCore/src/prog8/code/target/VMTarget.kt | 1 + .../prog8/codegen/m68k/M68kCodeGenerator.kt | 4 +-- compiler/src/prog8/compiler/Compiler.kt | 5 +++- compiler/test/TestStackFrameLocals.kt | 28 +++++++++++++++++++ 14 files changed, 64 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 401e646e6..4e22de8f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,7 +133,19 @@ Steps when encountering a compiler crash: - **NO EMOJI in user documentation**. Do not use emoji or decorative unicode symbols in documentation files. Functional unicode symbols are acceptable when they serve a clear purpose (e.g., → for arrows, ± for plus-minus, × for multiplication, ° for degrees). Avoid decorative emoji like ❌ ✅ ⚠️ 🎉 etc. ### Code Style Guidelines -**Minimal comments when making changes**: When modifying existing code, add only essential comments that explain *why* a change was made or document non-obvious behavior. **Do not add verbose comments** that restate what the code does; let the code speak for itself. Existing extensive comments should be preserved, but new changes should have minimal commentary. +**Code comments**: Comments are the exception, not the norm. A comment earns its place only by recording +something the code itself cannot say: the reason for a non-obvious choice, or a constraint that +a well-meaning future edit would otherwise violate. + +- Inline comment inside a function body: 1 line, never more. +- KDoc on a declaration: 3 lines, unless a genuine non-obvious contract cannot be stated shorter. +- Never restate the identifier, the type, or what the following line does. +- Prefer a good name or a well-named function over a comment that explains it. + +Before keeping any comment, delete it mentally and ask what the reader loses. If nothing, it goes. + +Keep existing comments as they are; do not expand, reword, or reformat them while editing +nearby lines - unless that would cause inconsistency. **Kotlin string `$` handling**: The `$` character is special in Kotlin string literals (string interpolation). If you need a literal `$` character, use Kotlin's `$$"..."` interpolation prefix instead of backslash escaping (`\$`) or `${'$'}`. The `$$"..."` syntax is a fairly recent Kotlin feature that avoids nasty escaping. For example: ```kotlin diff --git a/codeCore/src/prog8/code/core/ICompilationTarget.kt b/codeCore/src/prog8/code/core/ICompilationTarget.kt index a0b3359a3..b6aee5171 100644 --- a/codeCore/src/prog8/code/core/ICompilationTarget.kt +++ b/codeCore/src/prog8/code/core/ICompilationTarget.kt @@ -87,6 +87,13 @@ interface ICompilationTarget: IStringEncoding, IMemSizer { val defaultLauncherType: CbmPrgLauncherType val supportsBankedCalls: Boolean + /** + * True when the code generator allocates subroutine locals and per-activation intermediate values + * in a stack frame instead of static memory, so a subroutine on a call-graph cycle can have more + * than one live activation. False means two live activations would share the same storage. + */ + val supportsStackFrames: Boolean + fun initializeMemoryAreas(compilerOptions: CompilationOptions) fun getFloatAsmBytes(num: Number): String diff --git a/codeCore/src/prog8/code/target/Amiga1200Target.kt b/codeCore/src/prog8/code/target/Amiga1200Target.kt index 8006ff752..ab2c2cdef 100644 --- a/codeCore/src/prog8/code/target/Amiga1200Target.kt +++ b/codeCore/src/prog8/code/target/Amiga1200Target.kt @@ -12,6 +12,7 @@ class Amiga1200Target: ICompilationTarget, override val name = NAME override val supportsBankedCalls = true + override val supportsStackFrames = true override val defaultEncoding = Encoding.ISO override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/Amiga500Target.kt b/codeCore/src/prog8/code/target/Amiga500Target.kt index 541e21ddb..c5c6dca38 100644 --- a/codeCore/src/prog8/code/target/Amiga500Target.kt +++ b/codeCore/src/prog8/code/target/Amiga500Target.kt @@ -12,6 +12,7 @@ class Amiga500Target: ICompilationTarget, override val name = NAME override val supportsBankedCalls = true + override val supportsStackFrames = true override val defaultEncoding = Encoding.ISO override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/C128Target.kt b/codeCore/src/prog8/code/target/C128Target.kt index 7e9038303..749bcba88 100644 --- a/codeCore/src/prog8/code/target/C128Target.kt +++ b/codeCore/src/prog8/code/target/C128Target.kt @@ -12,6 +12,7 @@ class C128Target: ICompilationTarget, override val name = NAME override val supportsBankedCalls = true + override val supportsStackFrames = false override val defaultEncoding = Encoding.PETSCII override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/C64Target.kt b/codeCore/src/prog8/code/target/C64Target.kt index 5586b5299..73aa97193 100644 --- a/codeCore/src/prog8/code/target/C64Target.kt +++ b/codeCore/src/prog8/code/target/C64Target.kt @@ -13,6 +13,7 @@ class C64Target: ICompilationTarget, override val name = NAME override val supportsBankedCalls = true + override val supportsStackFrames = false override val defaultEncoding = Encoding.PETSCII override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/ConfigFileTarget.kt b/codeCore/src/prog8/code/target/ConfigFileTarget.kt index e611d4892..f68e26377 100644 --- a/codeCore/src/prog8/code/target/ConfigFileTarget.kt +++ b/codeCore/src/prog8/code/target/ConfigFileTarget.kt @@ -19,6 +19,7 @@ class ConfigFileTarget( override val supportsBankedCalls: Boolean, override val defaultEncoding: Encoding, override val cpu: CpuType, + override val supportsStackFrames: Boolean, override val PROGRAM_LOAD_ADDRESS: UInt, override val PROGRAM_MEMTOP_ADDRESS: UInt, override val BSSHIGHRAM_START: UInt, @@ -144,6 +145,7 @@ class ConfigFileTarget( props.getProperty("supports_banked_calls", "false").toBoolean(), Encoding.entries.first { it.prefix==props.getString("encoding") }, cpuType, + props.getProperty("supports_stack_frames", cpuType.is68k.toString()).toBoolean(), props.getInteger("load_address"), props.getInteger("memtop"), props.getInteger("bss_highram_start"), diff --git a/codeCore/src/prog8/code/target/Cx16Target.kt b/codeCore/src/prog8/code/target/Cx16Target.kt index d509cb42d..578afdfff 100644 --- a/codeCore/src/prog8/code/target/Cx16Target.kt +++ b/codeCore/src/prog8/code/target/Cx16Target.kt @@ -12,6 +12,7 @@ class Cx16Target: ICompilationTarget, override val name = NAME override val supportsBankedCalls = true + override val supportsStackFrames = false override val defaultEncoding = Encoding.PETSCII override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/PETTarget.kt b/codeCore/src/prog8/code/target/PETTarget.kt index daad58ee2..aeb64419a 100644 --- a/codeCore/src/prog8/code/target/PETTarget.kt +++ b/codeCore/src/prog8/code/target/PETTarget.kt @@ -12,6 +12,7 @@ class PETTarget: ICompilationTarget, override val name = NAME override val supportsBankedCalls = false + override val supportsStackFrames = false override val defaultEncoding = Encoding.PETSCII override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/Qemu68kTarget.kt b/codeCore/src/prog8/code/target/Qemu68kTarget.kt index 54c546618..fe2669356 100644 --- a/codeCore/src/prog8/code/target/Qemu68kTarget.kt +++ b/codeCore/src/prog8/code/target/Qemu68kTarget.kt @@ -16,6 +16,7 @@ class Qemu68kTarget: ICompilationTarget, override val name = NAME override val supportsBankedCalls = false + override val supportsStackFrames = true override val defaultEncoding = Encoding.ISO override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeCore/src/prog8/code/target/VMTarget.kt b/codeCore/src/prog8/code/target/VMTarget.kt index 203892993..75ceb2334 100644 --- a/codeCore/src/prog8/code/target/VMTarget.kt +++ b/codeCore/src/prog8/code/target/VMTarget.kt @@ -12,6 +12,7 @@ class VMTarget: ICompilationTarget, override val name = NAME override val supportsBankedCalls = false + override val supportsStackFrames = false override val defaultEncoding = Encoding.ISO override val libraryPath = null override val customLauncher = emptyList() diff --git a/codeGenM68k/src/prog8/codegen/m68k/M68kCodeGenerator.kt b/codeGenM68k/src/prog8/codegen/m68k/M68kCodeGenerator.kt index 4d0533141..6eaa4a065 100644 --- a/codeGenM68k/src/prog8/codegen/m68k/M68kCodeGenerator.kt +++ b/codeGenM68k/src/prog8/codegen/m68k/M68kCodeGenerator.kt @@ -29,8 +29,8 @@ class M68kCodeGenerator(val retainSSA: Boolean): ICodeGeneratorBackend { } irProgram.verifyRegisterTypes(virtualRegisterTypes) - // Vertical Slice Prototype: stack-frame locals for frameable leaf subroutines - StackFrameLayout(irProgram, errors).apply() + if (options.compTarget.supportsStackFrames) + StackFrameLayout(irProgram, errors).apply() if (options.dumpVariables) dumpVariables(irProgram) diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 741c2dc5f..5a17b2426 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -717,7 +717,10 @@ private fun postprocessAst(program: Program, errors: IErrorReporter, compilerOpt errors.report() program.variousCleanups(errors, compilerOptions) val callGraph = CallGraph(program) - callGraph.checkRecursiveCalls(errors) + // on a stack-frame target a recursive subroutine either gets per-activation storage or is + // rejected by StackFrameLayout, so there is nothing left to warn about here + if(!compilerOptions.compTarget.supportsStackFrames) + callGraph.checkRecursiveCalls(errors) if(compilerOptions.compTarget.cpu.is68k) { checkDeferInReentrantSubroutines(program, callGraph, errors) checkFrameAddressEscapes(program, errors) diff --git a/compiler/test/TestStackFrameLocals.kt b/compiler/test/TestStackFrameLocals.kt index 90640e578..b76e57dd8 100644 --- a/compiler/test/TestStackFrameLocals.kt +++ b/compiler/test/TestStackFrameLocals.kt @@ -530,6 +530,34 @@ main { otherErrors.errors.size shouldBe 0 } + test("recursion warning is only emitted for targets without stack frames") { + val src = """ +main { + sub rec(ubyte n) -> ubyte { + if n == 0 { + return 0 + } + return rec(n-1) + 1 + } + sub start() { + g = rec(3) + } + ubyte @shared g +} +""" + val m68kErrors = ErrorReporterForTests(keepMessagesAfterReporting = true) + compileText(Qemu68kTarget(), optimize = false, src, tempdir().toPath(), + writeAssembly = false, assemble = false, errors = m68kErrors) shouldNotBe null + m68kErrors.errors.size shouldBe 0 + m68kErrors.warnings.any { it.contains("recursive subroutine") } shouldBe false + + val otherErrors = ErrorReporterForTests(keepMessagesAfterReporting = true) + compileText(C64Target(), optimize = false, src, tempdir().toPath(), + writeAssembly = false, assemble = false, errors = otherErrors) shouldNotBe null + otherErrors.errors.size shouldBe 0 + otherErrors.warnings.any { it.contains("recursive subroutine") } shouldBe true + } + test("slice 3: defer in a recursive subroutine is rejected on m68k only") { val src = """ main {