diff --git a/docs/compiler/pbs/agendas/18.5. Ignored Call Results in Executable Lowering Agenda.md b/docs/compiler/pbs/agendas/18.5. Ignored Call Results in Executable Lowering Agenda.md new file mode 100644 index 00000000..fae03003 --- /dev/null +++ b/docs/compiler/pbs/agendas/18.5. Ignored Call Results in Executable Lowering Agenda.md @@ -0,0 +1,127 @@ +# Ignored Call Results in Executable Lowering Agenda + +## Status + +Open + +## Domain Owner + +`docs/compiler/pbs` + +Este tema pertence ao domínio PBS do compiler porque afeta: + +- semântica prática de `expression statement`; +- lowering executável para `IRBackend`/`IRVM`; +- ergonomia de uso de SDKs e hosts que retornam status; +- compatibilidade entre o que o código-fonte parece permitir e o que o pipeline realmente aceita. + +## Problema + +Hoje uma chamada com retorno usada como statement isolado pode deixar valor sobrando na stack no lowering executável. + +Exemplo real: + +```pbs +Gfx.set_sprite(...); +``` + +No estado atual, isso compila no frontend, mas pode falhar no pipeline backend com erro de validação de stack no `RET` da função, porque o resultado da call não foi consumido explicitamente. + +Isso cria uma inconsistência operacional: + +1. o código parece válido para o autor; +2. o frontend aceita a forma; +3. o backend exige, na prática, um consumo manual do retorno, por exemplo: + +```pbs +let sprite_status = Gfx.set_sprite(...); +``` + +## Contexto + +O problema apareceu ao migrar o consumo de sprite em PBS para o contrato novo de `Gfx.set_sprite`, que retorna um status inteiro. + +Os pontos observados no código atual são: + +- `ExpressionStatement` faz lowering apenas da expressão; +- o lowering de `CallExpr` emite `CALL_HOST`, `CALL_FUNC` ou `CALL_INTRINSIC` com `retSlots`; +- não há descarte automático do valor quando a expressão é usada apenas como statement; +- o validador de `IRVM` exige que funções `void` retornem com stack height `0`. + +Na prática, isso transforma "ignorar retorno" em comportamento parcialmente suportado pela linguagem, mas não suportado pelo pipeline executável. + +## Opções + +### Opção A + +Manter o comportamento atual e exigir consumo explícito de todo retorno em PBS. + +### Opção B + +Permitir que `expression statements` descartem automaticamente o resultado quando a expressão produzir valor. + +### Opção C + +Permitir descarte automático apenas para um subconjunto de calls, como hosts/SDKs anotados como status descartável. + +## Tradeoffs + +### Opção A + +- Prós: + - regra simples no backend; + - evita descarte implícito sem intenção. +- Contras: + - ergonomia ruim para APIs baseadas em status; + - surpresa para autores, porque `foo();` parece natural mas falha mais tarde; + - expõe detalhe de stack do backend ao código-fonte. + +### Opção B + +- Prós: + - comportamento esperado para statement de expressão; + - reduz ruído em código de jogo e SDK; + - alinha parsing, semântica prática e lowering executável. +- Contras: + - introduz descarte implícito; + - exige regra clara para saber quando emitir `POP`. + +### Opção C + +- Prós: + - mais controle semântico; + - evita descarte implícito amplo. +- Contras: + - adiciona complexidade de contrato e metadata; + - mistura política de produto/API dentro do lowering; + - não resolve a expectativa geral sobre `expression statement`. + +## Recomendação + +Seguir com a **Opção B**. + +Direção recomendada: + +1. `expression statement` deve ser permitido mesmo quando a expressão produzir valor; +2. o lowering executável deve emitir descarte explícito do resultado não usado; +3. a regra deve ser geral para expressões com resultado materializado em stack, não especial para `Gfx.set_sprite`; +4. testes de regressão devem cobrir host calls, callable calls e intrinsics com retorno ignorado. + +Essa direção preserva a ergonomia esperada da linguagem e remove um vazamento indevido do modelo de stack do backend para o código PBS. + +## Perguntas em Aberto + +1. O descarte automático deve valer para qualquer `expression statement` com valor ou apenas para formas lowerables em v1? +2. O frontend semântico deve emitir algum warning opcional para retorno ignorado, ou isso fica fora do escopo atual? +3. O descarte deve acontecer somente no lowering executável ou também virar regra explícita de spec para statements? +4. Existe algum caso em que o descarte automático possa mascarar bug real de usuário que hoje seria detectado mais cedo? + +## Próximo Passo Sugerido + +Converter esta agenda em uma `decision` do domínio `compiler/pbs` fechando a política para `expression statement` com resultado ignorado. + +Depois disso, abrir um `pull-request/plan` curto para: + +1. ajustar o lowering de `ExpressionStatement`; +2. adicionar fixtures e testes de regressão no frontend/backend pipeline; +3. propagar a regra para specs relevantes de statements e lowering. diff --git a/docs/compiler/pbs/agendas/README.md b/docs/compiler/pbs/agendas/README.md index f2808ef7..8a9ad6cb 100644 --- a/docs/compiler/pbs/agendas/README.md +++ b/docs/compiler/pbs/agendas/README.md @@ -11,6 +11,7 @@ Closed agendas are moved to `docs/pbs/agendas/archive`. 3. `18.2. Backend Workshop 2 - LowerToIRVM and IRVM Contract.md` 4. `18.3. Backend Workshop 3 - Bytecode Marshaling and Runtime Conformance.md` 5. `18.4. Asset References in Game Code - Names vs Compile-Time Lowering Agenda.md` +6. `18.5. Ignored Call Results in Executable Lowering Agenda.md` ## Purpose diff --git a/docs/packer/agendas/README.md b/docs/packer/agendas/README.md index cfc82d71..73911669 100644 --- a/docs/packer/agendas/README.md +++ b/docs/packer/agendas/README.md @@ -7,6 +7,7 @@ This directory contains active packer discussion agendas. 1. [`Tilemap and Metatile Runtime Binary Layout Agenda.md`](./Tilemap%20and%20Metatile%20Runtime%20Binary%20Layout%20Agenda.md) 2. [`Pack Wizard Studio Decision Propagation Agenda.md`](./Pack%20Wizard%20Studio%20Decision%20Propagation%20Agenda.md) 3. [`Tile Bank Packing Materialization Agenda.md`](./Tile%20Bank%20Packing%20Materialization%20Agenda.md) +4. [`Variable Tile Bank Palette Serialization Agenda.md`](./Variable%20Tile%20Bank%20Palette%20Serialization%20Agenda.md) The first packer agenda wave was consolidated into normative specs under [`../specs/`](../specs/) and didactic material under [`../learn/`](../learn/). diff --git a/docs/packer/agendas/Variable Tile Bank Palette Serialization Agenda.md b/docs/packer/agendas/Variable Tile Bank Palette Serialization Agenda.md new file mode 100644 index 00000000..bb1a1c9a --- /dev/null +++ b/docs/packer/agendas/Variable Tile Bank Palette Serialization Agenda.md @@ -0,0 +1,142 @@ +# Variable Tile Bank Palette Serialization Agenda + +## Status + +Active + +## Domain Owner + +- `docs/packer` + +Cross-domain dependency: + +- `../runtime` + +## Purpose + +Decide whether `tile bank` payloads should keep serializing a fixed block of `64` palettes in v1, or evolve to serialize only the palettes actually authored by the asset. + +## Context + +The current `tile bank` contract is aligned between `packer` and `runtime`: + +- pixels are serialized as packed `u4`; +- palettes are serialized as `RGB565 u16`; +- `palette_count = 64`; +- payload size is derived from `ceil(width * height / 2) + (64 * 16 * 2)`. + +This means a `tile bank` with `1` authored palette still serializes `64` palette slots. Unused slots are currently zero-filled, which is valid for the runtime, but wastes cartridge space. + +The packer now exposes: + +- `metadata.palette_count = 64` +- `metadata.palette_authored = ` + +This makes the waste explicit, but does not remove it. + +## Problem + +The runtime-facing v1 contract favors fixed-size decode simplicity over cartridge efficiency. + +We need to decide whether: + +1. fixed `64` palette slots remain the normative contract for `tile bank`; +2. `palette_count` becomes variable and payload size shrinks to the authored set; +3. another staged strategy is better. + +## Options + +### Option A: Keep Fixed 64-Palette Serialization + +Keep the current contract unchanged: + +- `palette_count` remains `64`; +- every `tile bank` serializes `64 * 16 * 2 = 2048` bytes of palettes; +- authored palettes populate their indexed slots; +- unused slots are zero-filled. + +### Option B: Move to Variable Palette Serialization + +Evolve the contract so `tile bank` serializes only authored palettes: + +- `palette_count` becomes the authored count; +- payload size becomes `ceil(width * height / 2) + (palette_count * 16 * 2)`; +- runtime validates and decodes variable palette blocks. + +### Option C: Keep V1 Fixed, Design Variable Serialization for V2 + +Preserve the current runtime-facing v1 contract now, but explicitly open a v2 format/contract: + +- v1 stays fixed at `64`; +- v2 introduces variable palette serialization; +- packer and runtime keep clear compatibility boundaries. + +## Tradeoffs + +### Option A + +Pros: + +- no further runtime work; +- simplest decode path; +- current specs and tests stay valid. + +Cons: + +- every `tile bank` pays the full 2 KB palette cost; +- cartridge size is inflated for small authored palette sets; +- the metadata now exposes a mismatch between authored and serialized counts by design. + +### Option B + +Pros: + +- reduces cartridge size; +- makes `palette_count` semantically tighter; +- removes fixed empty palette padding from the payload. + +Cons: + +- requires coordinated changes in `packer`, `runtime`, specs, and tests; +- changes the `size` / `decoded_size` formulas; +- raises compatibility and migration questions for existing assets and cartridges. + +### Option C + +Pros: + +- keeps current work stable; +- makes room for a cleaner future contract; +- avoids mixing optimization work into the current tile-bank rollout. + +Cons: + +- known waste remains in v1; +- adds future format/version management work. + +## Recommendation + +Recommendation for now: `Option C`. + +Reasoning: + +- the current fixed-size contract is already implemented and aligned; +- the waste is real, but it is not a correctness bug; +- changing it now would reopen a cross-domain runtime contract that was just stabilized; +- if we want variable palette serialization, it should be introduced deliberately as a versioned follow-up, not as an incidental tweak. + +## Open Questions + +1. Should variable palette serialization be introduced as a new `tile bank` payload version, or as an incompatible change to the current one? +2. If `palette_count` becomes variable, should runtime still materialize a 64-slot bank in memory, or truly shrink the resident representation too? +3. Should palette slot identity remain sparse by authored `index`, or should serialization canonicalize into a dense runtime block? +4. Which domain owns the compatibility story for existing cartridges: `packer`, `runtime`, or `shipper`? + +## Expected Follow-up + +If the team wants to eliminate the fixed 64-palette padding: + +1. create a corresponding runtime agenda/decision; +2. define the versioning and compatibility story; +3. update `packer` and `runtime` specs together; +4. then plan code changes in both domains. diff --git a/prometeu-compiler/frontends/prometeu-frontend-pbs/src/main/resources/stdlib/1/sdk/gfx/main.pbs b/prometeu-compiler/frontends/prometeu-frontend-pbs/src/main/resources/stdlib/1/sdk/gfx/main.pbs index a3f6edd9..8fb80fea 100644 --- a/prometeu-compiler/frontends/prometeu-frontend-pbs/src/main/resources/stdlib/1/sdk/gfx/main.pbs +++ b/prometeu-compiler/frontends/prometeu-frontend-pbs/src/main/resources/stdlib/1/sdk/gfx/main.pbs @@ -28,7 +28,7 @@ declare host LowGfx { [Host(module = "gfx", name = "set_sprite", version = 1)] [Capability(name = "gfx")] fn set_sprite( - asset_name: str, + bank_id: int, index: int, x: int, y: int, @@ -82,7 +82,7 @@ declare service Gfx } fn set_sprite( - asset_name: str, + bank_id: int, index: int, x: int, y: int, @@ -94,7 +94,7 @@ declare service Gfx priority: int ) -> int { - return LowGfx.set_sprite(asset_name, index, x, y, tile_id, palette_id, active, flip_x, flip_y, priority); + return LowGfx.set_sprite(bank_id, index, x, y, tile_id, palette_id, active, flip_x, flip_y, priority); } fn draw_text(x: int, y: int, message: str, color: Color) -> void diff --git a/prometeu-compiler/frontends/prometeu-frontend-pbs/src/test/java/p/studio/compiler/services/PBSFrontendPhaseServiceTest.java b/prometeu-compiler/frontends/prometeu-frontend-pbs/src/test/java/p/studio/compiler/services/PBSFrontendPhaseServiceTest.java index cba2b53e..432df120 100644 --- a/prometeu-compiler/frontends/prometeu-frontend-pbs/src/test/java/p/studio/compiler/services/PBSFrontendPhaseServiceTest.java +++ b/prometeu-compiler/frontends/prometeu-frontend-pbs/src/test/java/p/studio/compiler/services/PBSFrontendPhaseServiceTest.java @@ -627,6 +627,57 @@ class PBSFrontendPhaseServiceTest { assertTrue(irBackend.getExecutableFunctions().stream().anyMatch(function -> "frame".equals(function.callableName()))); } + @Test + void shouldLowerSdkGfxSetSpriteFacadeUsingBankIdContract() throws IOException { + final var projectRoot = tempDir.resolve("project-bootstrap-sdk-gfx-set-sprite"); + final var sourceRoot = projectRoot.resolve("src"); + final var modulePath = sourceRoot.resolve("app"); + Files.createDirectories(modulePath); + + final var sourceFile = modulePath.resolve("source.pbs"); + final var modBarrel = modulePath.resolve("mod.barrel"); + Files.writeString(sourceFile, """ + import { Gfx } from @sdk:gfx; + + fn frame() -> int + { + return Gfx.set_sprite(2, 0, 12, 18, 7, 3, true, false, true, 1); + } + """); + Files.writeString(modBarrel, "pub fn frame() -> int;"); + + final var projectTable = new ProjectTable(); + final var fileTable = new FileTable(1); + final var projectId = projectTable.register(ProjectDescriptor.builder() + .rootPath(projectRoot) + .name("app") + .version("1.0.0") + .sourceRoots(ReadOnlyList.wrap(List.of(sourceRoot))) + .build()); + + registerFile(projectId, projectRoot, sourceFile, fileTable); + registerFile(projectId, projectRoot, modBarrel, fileTable); + + final var ctx = new FrontendPhaseContext( + projectTable, + fileTable, + new BuildStack(ReadOnlyList.wrap(List.of(projectId))), + 1); + final var diagnostics = DiagnosticSink.empty(); + + final var irBackend = new PBSFrontendPhaseService().compile( + ctx, + diagnostics, + LogAggregator.empty(), + BuildingIssueSink.empty()); + + assertTrue(diagnostics.stream().noneMatch(d -> + d.getCode().equals(PbsSemanticsErrors.E_SEM_EXEC_LOWERING_UNRESOLVED_CALLEE.name()))); + assertTrue(irBackend.getExecutableFunctions().stream().anyMatch(function -> "frame".equals(function.callableName()))); + assertTrue(irBackend.getReservedMetadata().hostMethodBindings().stream() + .anyMatch(h -> h.ownerName().equals("LowGfx") && h.sourceMethodName().equals("set_sprite"))); + } + @Test void shouldReportInconsistentBarrelInsideSdkBootstrapFixture() throws IOException { final var projectRoot = tempDir.resolve("project-stdlib-barrel-inconsistent"); diff --git a/prometeu-compiler/prometeu-build-pipeline/build.gradle.kts b/prometeu-compiler/prometeu-build-pipeline/build.gradle.kts index 393bd0e4..c590da1f 100644 --- a/prometeu-compiler/prometeu-build-pipeline/build.gradle.kts +++ b/prometeu-compiler/prometeu-build-pipeline/build.gradle.kts @@ -8,4 +8,12 @@ dependencies { implementation(project(":prometeu-compiler:prometeu-compiler-core")) implementation(project(":prometeu-compiler:prometeu-frontend-api")) implementation(project(":prometeu-compiler:prometeu-frontend-registry")) -} \ No newline at end of file +} + +tasks.register("runCompile") { + group = "application" + description = "Runs p.studio.compiler.Compile against test-projects/main." + classpath = sourceSets.main.get().runtimeClasspath + mainClass = "p.studio.compiler.Compile" + workingDir = rootDir +} diff --git a/test-projects/main/cartridge/assets.pa b/test-projects/main/cartridge/assets.pa index e69de29b..b29c05c9 100644 Binary files a/test-projects/main/cartridge/assets.pa and b/test-projects/main/cartridge/assets.pa differ diff --git a/test-projects/main/cartridge/manifest.json b/test-projects/main/cartridge/manifest.json index 4f93d84a..901fe9e6 100644 --- a/test-projects/main/cartridge/manifest.json +++ b/test-projects/main/cartridge/manifest.json @@ -6,5 +6,5 @@ "app_version": "0.1.0", "app_mode": "Game", "entrypoint": "0", - "capabilities": ["log", "gfx"] + "capabilities": ["log", "gfx", "asset"] } diff --git a/test-projects/main/cartridge/program.pbx b/test-projects/main/cartridge/program.pbx index a090a149..222af36f 100644 Binary files a/test-projects/main/cartridge/program.pbx and b/test-projects/main/cartridge/program.pbx differ diff --git a/test-projects/main/run.sh b/test-projects/main/run.sh index f840678a..3f792c15 100755 --- a/test-projects/main/run.sh +++ b/test-projects/main/run.sh @@ -2,5 +2,6 @@ set -e +cp build/assets.pa cartridge cp build/program.pbx cartridge -./runtime/prometeu run cartridge \ No newline at end of file +./runtime/prometeu run cartridge diff --git a/test-projects/main/src/main.pbs b/test-projects/main/src/main.pbs index cd6c7fb9..f39b9981 100644 --- a/test-projects/main/src/main.pbs +++ b/test-projects/main/src/main.pbs @@ -9,7 +9,8 @@ fn frame() -> void let touch = Input.touch(); Gfx.clear(new Color(6577)); - Gfx.draw_square(touch.x() - 8, touch.y() - 8, 16, 16, new Color(65535), new Color(13271)); + // Gfx.draw_square(touch.x() - 8, touch.y() - 8, 16, 16, new Color(65535), new Color(13271)); + let sprite_status = Gfx.set_sprite(0, 0, touch.x() - 8, touch.y() - 8, 0, 1, true, false, false, 0); let a = 10; let b = 15;