From d6cf7cfa440033968432bde3190290a9df761e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Sun, 20 Sep 2026 19:14:43 +0200 Subject: [PATCH] fix(format): keep statement separators and canonical macro bodies Assisted-by: GLM 5.3 Flash --- format/format.go | 22 +++-- format/format_test.go | 181 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 6 deletions(-) diff --git a/format/format.go b/format/format.go index 624f193..fbdbc52 100644 --- a/format/format.go +++ b/format/format.go @@ -254,11 +254,12 @@ func renderPreproc(line []token.Token) string { line[2].Kind == token.String { return "#include " + line[2].Text } - parts := make([]string, 0, len(line)-1) - for _, t := range line[1:] { - parts = append(parts, t.Text) - } - return "#" + strings.Join(parts, " ") + // The body of a directive, a macro definition included, is an ordinary + // token run: rendering it through renderOps applies the same punctuation + // rules as everywhere else, so a macro body keeps its canonical spelling + // ($v, (a, b), the ';' separators between statements) instead of being + // spread with a space between every token. + return "#" + renderOps(line[1:]) } // renderOps re-spaces a run of operand tokens into canonical form. It never @@ -348,6 +349,13 @@ func spaceBetween(prev, cur token.Token) bool { return false case token.Comma: return false + case token.Semicolon: + // A ';' is a statement separator on the assembly path, not an + // operand: dropping it would fuse two statements into a line the + // assembler rejects, so it must survive as punctuation. It glues + // to the statement it ends and the next statement takes one space, + // matching the toolchain's listing style. + return false case token.Star, token.Plus, token.Minus, token.Slash, token.Pipe: return false case token.LShift, token.RShift, token.Arrow, token.At: @@ -376,7 +384,9 @@ func spaceBetween(prev, cur token.Token) bool { return false case token.LAngle, token.RAngle: return false - case token.Comma: + case token.Comma, token.Semicolon: + // The statement after a ';' separator takes its own space, exactly + // like the operand after a comma. return true } return true diff --git a/format/format_test.go b/format/format_test.go index cbcdb65..c8de510 100644 --- a/format/format_test.go +++ b/format/format_test.go @@ -5,9 +5,11 @@ package format import ( "os" + "slices" "strings" "testing" + "sourcedock.dev/petrbalvin/gasm-devkit/ast" "sourcedock.dev/petrbalvin/gasm-devkit/lexer" "sourcedock.dev/petrbalvin/gasm-devkit/parser" "sourcedock.dev/petrbalvin/gasm-devkit/token" @@ -298,6 +300,185 @@ func TestCRLFInputIsNormalisedToLF(t *testing.T) { } } +// TestSemicolonSeparators pins the treatment of ';' statement separators. +// The separator is load-bearing on the assembly path, where the parser reads +// semicolon-separated statements: a formatter that drops it fuses two +// statements into a line the assembler rejects, which is data corruption. +// Each row pins the canonical spelling, one space after the ';', tight +// before it, the way the toolchain's own sources and listings write it. +func TestSemicolonSeparators(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + { + name: "between instructions, tight", + in: "TEXT ·f(SB), $0\nBYTE $0x48;BYTE $0xc7\nRET\n", + want: "TEXT ·f(SB), $0\n\tBYTE $0x48; BYTE $0xc7\n\tRET\n", + }, + { + name: "between instructions, spaced", + in: "TEXT ·f(SB), $0\nBYTE $0x48 ; BYTE $0xc7\nRET\n", + want: "TEXT ·f(SB), $0\n\tBYTE $0x48; BYTE $0xc7\n\tRET\n", + }, + { + name: "after a label", + in: "TEXT ·f(SB), $0\nlabel: BYTE $1; BYTE $2\nRET\n", + want: "TEXT ·f(SB), $0\nlabel:\n\tBYTE $1; BYTE $2\n\tRET\n", + }, + { + // The continuation-spliced macro shape of the runtime sources: + // the lexer makes one logical line of the backslash continuations. + name: "inside a macro body, continued", + in: "#define MOVLTOREG(v, off) \\\n\tMOVL $v, AX; \\\n\tMOVL AX, ret+off(FP)\n", + want: "#define MOVLTOREG(v, off) MOVL $v, AX; MOVL AX, ret+off(FP)\n", + }, + { + name: "inside a macro body, one line", + in: "#define PEAS BYTE $0x0a; BYTE $0x0b\n", + want: "#define PEAS BYTE $0x0a; BYTE $0x0b\n", + }, + { + name: "several separators in one line", + in: "TEXT ·f(SB), $0\nBYTE $1; BYTE $2; BYTE $3\nRET\n", + want: "TEXT ·f(SB), $0\n\tBYTE $1; BYTE $2; BYTE $3\n\tRET\n", + }, + { + name: "two separators back to back", + in: "TEXT ·f(SB), $0\nBYTE $1;; BYTE $2\nRET\n", + want: "TEXT ·f(SB), $0\n\tBYTE $1;; BYTE $2\n\tRET\n", + }, + { + name: "inside a line comment, untouched", + in: "TEXT ·f(SB), $0\n// keep; the; separators\nBYTE $1\nRET\n", + want: "TEXT ·f(SB), $0\n\t// keep; the; separators\n\tBYTE $1\n\tRET\n", + }, + { + name: "after a statement, before a comment", + in: "TEXT ·f(SB), $0\nMOVQ AX, BX; // tail\nRET\n", + want: "TEXT ·f(SB), $0\n\tMOVQ AX, BX; // tail\n\tRET\n", + }, + { + name: "last character on a line", + in: "TEXT ·f(SB), $0\nBYTE $1;\nRET\n", + want: "TEXT ·f(SB), $0\n\tBYTE $1;\n\tRET\n", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := Source(tc.in) + if got != tc.want { + t.Fatalf("formatting mismatch:\n--- got ---\n%q\n--- want ---\n%q", got, tc.want) + } + if again := Source(got); again != got { + t.Fatalf("not idempotent:\n%q", again) + } + if in, out := strings.Count(tc.in, ";"), strings.Count(got, ";"); in != out { + t.Fatalf("semicolon count changed: %d -> %d\n%s", in, out, got) + } + if _, errs := parser.Parse("in.s", got); len(errs) > 0 { + t.Fatalf("formatted output no longer parses: %v", errs) + } + }) + } +} + +// TestSemicolonStatementRoundTrip proves the formatter's contract on the +// path where ';' separates statements: parse the source the way the +// assembler does, format it, re-parse the formatted text and compare the +// statement sequence. Raw operand texts are token-joined, so they are +// insensitive to the whitespace a format pass chooses, and the comparison +// can only fail when a token is lost: dropping a ';' fuses two statements +// into one, exactly the corruption the released formatter committed. +func TestSemicolonStatementRoundTrip(t *testing.T) { + src := "#define MOVLTOREG(v, off) \\\n" + + "\tMOVL $v, AX; \\\n" + + "\tMOVL AX, ret+off(FP)\n" + + "\n" + + "TEXT ·f(SB), NOSPLIT, $0\n" + + "BYTE $0x48; BYTE $0xc7\n" + + "first: BYTE $1; BYTE $2\n" + + "MOVLTOREG($42, 0)\n" + + "RET\n" + + before, errs := parser.ParseWithOptions("in.s", src, parser.Options{Expand: true}) + if len(errs) > 0 { + t.Fatalf("source does not parse: %v", errs) + } + formatted := Source(src) + after, errs := parser.ParseWithOptions("in.s", formatted, parser.Options{Expand: true}) + if len(errs) > 0 { + t.Fatalf("formatted source does not parse: %v", errs) + } + + want, got := stmtSignature(before), stmtSignature(after) + if !slices.Equal(got, want) { + t.Fatalf("statement sequence changed:\n--- before ---\n%q\n--- after ---\n%q", want, got) + } + if again := Source(formatted); again != formatted { + t.Fatalf("not idempotent:\n%q", again) + } + + // The two BYTE statements on the first line must stay two: one fused + // statement here is the exact defect this package once shipped. + var bytes []string + for _, stmt := range stmtSignature(after) { + if rest, ok := strings.CutPrefix(stmt, "instr BYTE "); ok { + bytes = append(bytes, rest) + } + } + if want := []string{"$ 0x48", "$ 0xc7", "$ 1", "$ 2"}; !slices.Equal(bytes, want) { + t.Fatalf("BYTE statements after expansion = %q, want %q", bytes, want) + } +} + +// stmtSignature flattens a parsed file into one string per declaration and +// statement, in source order. Every component is token-derived, so the +// signature is stable across format passes and moves only when a token is +// lost or gained. +func stmtSignature(f *ast.File) []string { + var out []string + for _, d := range f.Decls { + switch d := d.(type) { + case *ast.Text: + out = append(out, "text "+d.Name.Raw) + for _, s := range d.Body { + out = append(out, stmtText(s)) + } + case *ast.Globl: + out = append(out, "globl "+d.Name.Raw) + case *ast.Data: + out = append(out, "data "+d.Name.Raw) + case *ast.Include: + out = append(out, "include "+d.Header.Text) + case *ast.Preproc: + out = append(out, "preproc "+d.Raw) + } + } + for _, s := range f.Orphans { + out = append(out, stmtText(s)) + } + return out +} + +// stmtText renders one statement for stmtSignature. +func stmtText(s ast.Stmt) string { + switch s := s.(type) { + case *ast.Label: + return "label " + s.Name.Text + case *ast.Instr: + parts := make([]string, 0, len(s.Operands)+1) + parts = append(parts, s.Mnemonic.Text) + for _, op := range s.Operands { + parts = append(parts, op.Raw) + } + return "instr " + strings.Join(parts, " ") + default: + return "stmt" + } +} + // lexOperands lexes a single operand string and drops the EOF token. func lexOperands(s string) []token.Token { toks := lexer.Tokenize(s)