0xMiden / 0xMiden/miden-vm

Printed procedure signatures and `@locals` attributes are invalid Masm

Ouverte
#3,658 5 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Rust
Étoiles
772
Forks
352
Merge moyen
1 j 12 h
PR mergées (30 j)
93

Description

Formatting a module and parsing the result back fails for 5 files in
`crates/lib/core/asm` and for the 4 kernel `main.masm` / `main-alt.masm` fixtures,
from two separate causes.

#### Procedure signatures lose their parameter names

`crates/lib/core/asm/word.masm` declares:

pub proc reverse(input: word) -> word

which prints as:

pub proc reverse([felt; 4]) -> [felt; 4]

and parsing that back fails with `expected a named procedure parameter`. The same
signature parses once a name is present, so it is the printed form that is wrong.
Parameter names are not stored anywhere in the AST: `Procedure` holds an
`Option`, and `FunctionType` keeps only `params: SmallVec<[Type; 4]>`,
so the printer has no name to emit.

This affects `math/u64.masm`, `math/u128.masm`, `math/u256.masm`, `word.masm` and
`sys/vm/mod.masm`.

#### `@locals` is printed as a string

A procedure with locals prints as `@locals("4")`, which fails to parse with
`invalid procedure @locals attribute`, since that attribute takes an integer literal.

https://github.com/0xMiden/miden-vm/blob/5d97164493d2c441c306b9b2dbb1ad2fa67a2a3a/crates/assembly-syntax/src/ast/procedure/procedure.rs#L291

The `@callconv` attribute printed just below is quoted deliberately, since its
validation accepts either an identifier or a string, but `@locals` only accepts an
integer.

#### Proposed fix

For `@locals`, drop the quotes so the value prints as `@locals(4)`.

The signature case needs a decision first: either preserve parameter names in the
AST, or print placeholder names so the output at least parses.

The lossy parts of round-tripping are already documented on `test_roundtrip_formatting`,
but everything listed there still parses, whereas these two do not.
`parser_accepts_checked_in_masm_corpus` already walks the corpus, so asserting the
round-trip there would stop both from coming back.

#### Context

Same class as #3545, fixed in #3567.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.