From 151b268a592e273a994bf14c2de65f033071c877 Mon Sep 17 00:00:00 2001 From: Gulliby-development <20404543+Gulliby@users.noreply.github.com> Date: Thu, 5 Nov 2026 03:38:53 +0000 Subject: [PATCH] Add --min-saving to structpad suggest Made by a BAKED bench run (round 100). --- README.md | 16 ++++ src/structpad/cli.py | 30 ++++++-- src/structpad/render.py | 22 ++++-- tests/test_cli.py | 2 +- tests/test_min_saving.py | 160 +++++++++++++++++++++++++++++++++++++++ tests/test_readme.py | 14 ++++ 6 files changed, 232 insertions(+), 12 deletions(-) create mode 100644 tests/test_min_saving.py diff --git a/README.md b/README.md index 927e54b..f3dd2cd 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,22 @@ struct Frame: size 56 -> 48 (saves 8) bool ok; ``` +`--min-saving N` is available only with `suggest`; `--min-saving=N` is also supported and the default is 0. N is a whole number written with 1 to 6 decimal digits. A struct that would save fewer than N bytes is left out, so a packed struct and a struct that is already compact appear only when N is 0. When every struct is left out, the output is the one line `no struct saves at least N bytes` (`byte` when N is 1). + +```sh +structpad suggest --min-saving 8 net.sp +``` + +```text +struct Frame: size 56 -> 48 (saves 8) + struct Packet head; + u8 version; + struct Wire tail[2]; + bool ok; +``` + +An invalid value such as `-1` produces `invalid --min-saving "-1"`, and `--min-saving` with `layout` or `check` produces `layout does not take --min-saving` or `check does not take --min-saving`. Both exit with status 2. + Check structs against a padding limit; the default limit is zero bytes: ```sh diff --git a/src/structpad/cli.py b/src/structpad/cli.py index 33f1b8e..4b1728a 100644 --- a/src/structpad/cli.py +++ b/src/structpad/cli.py @@ -11,10 +11,11 @@ _USAGE = ( "usage: structpad layout [--abi lp64|ilp32] [--struct NAME]... [--format text|json] FILE\n" - " structpad suggest [--abi lp64|ilp32] FILE\n" + " structpad suggest [--abi lp64|ilp32] [--min-saving N] FILE\n" " structpad check [--abi lp64|ilp32] [--max-padding N] [--ignore NAME]... FILE\n" ) _MAX_PADDING_RE = re.compile(r"[0-9]{1,6}\Z") +_MIN_SAVING_RE = re.compile(r"[0-9]{1,6}\Z") _NAME_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]{0,63}\Z") @@ -29,10 +30,11 @@ def _write_error(stderr: BinaryIO, message: str) -> None: def _parse_options( command: str, arguments: list[str] -) -> tuple[str, int, str, tuple[str, ...], tuple[str, ...], str] | str: +) -> tuple[str, int, str, tuple[str, ...], tuple[str, ...], str, int] | str: abi = "lp64" output_format = "text" max_padding = 0 + min_saving = 0 ignored: list[str] = [] selected: list[str] = [] file_name: str | None = None @@ -90,6 +92,22 @@ def _parse_options( max_padding = int(value) continue + if argument == "--min-saving" or argument.startswith("--min-saving="): + if command != "suggest": + return f"{command} does not take --min-saving" + if argument == "--min-saving": + if index + 1 >= len(arguments): + return "--min-saving needs a value" + value = arguments[index + 1] + index += 2 + else: + value = argument[len("--min-saving=") :] + index += 1 + if _MIN_SAVING_RE.fullmatch(value) is None: + return f'invalid --min-saving "{_escape_controls(value)}"' + min_saving = int(value) + continue + if argument == "--ignore" or argument.startswith("--ignore="): if command != "check": return f"{command} does not take --ignore" @@ -131,7 +149,7 @@ def _parse_options( if file_name is None: return "missing FILE" - return abi, max_padding, file_name, tuple(ignored), tuple(selected), output_format + return abi, max_padding, file_name, tuple(ignored), tuple(selected), output_format, min_saving def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) -> int: @@ -152,7 +170,9 @@ def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) -> if isinstance(parsed_options, str): _write_error(stderr, parsed_options) return 2 - abi, max_padding, file_name, ignored_names, selected_names, output_format = parsed_options + abi, max_padding, file_name, ignored_names, selected_names, output_format, min_saving = ( + parsed_options + ) display = "" if file_name == "-" else file_name.replace("\\", "/") try: @@ -195,7 +215,7 @@ def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) -> stdout.write(renderer(structs, abi, selected_names or None)) return 0 if command == "suggest": - stdout.write(render_suggest(structs, abi)) + stdout.write(render_suggest(structs, abi, min_saving)) return 0 output = render_check(structs, abi, max_padding, ignored) diff --git a/src/structpad/render.py b/src/structpad/render.py index eb53b82..6a001ba 100644 --- a/src/structpad/render.py +++ b/src/structpad/render.py @@ -106,30 +106,40 @@ def render_layout_json( return (json.dumps(layouts, indent=2) + "\n").encode("utf-8") -def render_suggest(structs: Sequence[Struct], abi: str) -> bytes: - """Render stable field-order suggestions as UTF-8 bytes with LF endings.""" +def render_suggest(structs: Sequence[Struct], abi: str, min_saving: int = 0) -> bytes: + """Render stable field-order suggestions as UTF-8 bytes with LF endings. + + A struct that would save fewer than ``min_saving`` bytes is left out. + """ definitions = _definitions(structs) blocks: list[str] = [] for struct in structs: current = calculate_layout(struct, abi, definitions) if struct.packed: - blocks.append( - f"struct {struct.name}: packed, size {current.size}, nothing to reorder" - ) + if min_saving == 0: + blocks.append( + f"struct {struct.name}: packed, size {current.size}, nothing to reorder" + ) continue proposed = suggest(struct, abi, definitions) if proposed.fields == struct.fields: - blocks.append(f"struct {struct.name}: size {current.size}, already compact") + if min_saving == 0: + blocks.append(f"struct {struct.name}: size {current.size}, already compact") continue improved = calculate_layout(proposed, abi, definitions) saving = current.size - improved.size + if saving < min_saving: + continue lines = [ f"struct {struct.name}: size {current.size} -> {improved.size} (saves {saving})" ] lines.extend(f" {_field_declaration(field)}" for field in proposed.fields) blocks.append("\n".join(lines)) + if not blocks: + unit = "byte" if min_saving == 1 else "bytes" + return f"no struct saves at least {min_saving} {unit}\n".encode() return ("\n\n".join(blocks) + "\n").encode("utf-8") diff --git a/tests/test_cli.py b/tests/test_cli.py index c3a3899..072be7d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,7 +17,7 @@ _USAGE = ( b"usage: structpad layout [--abi lp64|ilp32] [--struct NAME]... [--format text|json] FILE\n" - b" structpad suggest [--abi lp64|ilp32] FILE\n" + b" structpad suggest [--abi lp64|ilp32] [--min-saving N] FILE\n" b" structpad check [--abi lp64|ilp32] [--max-padding N] [--ignore NAME]... FILE\n" ) _MIXED = ( diff --git a/tests/test_min_saving.py b/tests/test_min_saving.py new file mode 100644 index 0000000..db82af4 --- /dev/null +++ b/tests/test_min_saving.py @@ -0,0 +1,160 @@ +"""Example-based tests for the suggest command's minimum-saving option.""" + +import importlib +import sys +from io import BytesIO +from pathlib import Path + +import pytest + +_SOURCE_DIRECTORY = str(Path(__file__).resolve().parents[1] / "src") +if _SOURCE_DIRECTORY not in sys.path: + sys.path.insert(0, _SOURCE_DIRECTORY) +run = importlib.import_module("structpad.cli").run + +_NET_SOURCE = ( + b"struct Packet {\n" + b" u8 tag;\n" + b" u32 length;\n" + b" u16 flags;\n" + b" char name[10];\n" + b" ptr next;\n" + b"}\n" + b"\n" + b"packed struct Wire {\n" + b" u8 kind;\n" + b" u32 value;\n" + b"}\n" + b"\n" + b"struct Frame {\n" + b" u8 version;\n" + b" struct Packet head;\n" + b" struct Wire tail[2];\n" + b" bool ok;\n" + b"}\n" +) +_TWO_SAVINGS = ( + b"struct Small {\n" + b" u8 a;\n" + b" u16 b;\n" + b" u8 c;\n" + b" u16 d;\n" + b"}\n" + b"\n" + b"struct Big {\n" + b" u8 a;\n" + b" u64 b;\n" + b" u8 c;\n" + b" u64 d;\n" + b"}\n" +) +_FRAME_BLOCK = ( + b"struct Frame: size 56 -> 48 (saves 8)\n" + b" struct Packet head;\n" + b" u8 version;\n" + b" struct Wire tail[2];\n" + b" bool ok;\n" +) + + +def _run(args: list[str], source: bytes = _NET_SOURCE) -> tuple[int, bytes, bytes]: + stdout = BytesIO() + stderr = BytesIO() + status = run(args, BytesIO(source), stdout, stderr) + return status, stdout.getvalue(), stderr.getvalue() + + +def test_default_output_is_unchanged() -> None: + status, default_out, stderr = _run(["suggest", "-"]) + assert status == 0 + assert stderr == b"" + assert default_out == ( + b"struct Packet: size 32, already compact\n" + b"\n" + b"struct Wire: packed, size 5, nothing to reorder\n" + b"\n" + _FRAME_BLOCK + ) + assert _run(["suggest", "--min-saving", "0", "-"]) == (0, default_out, b"") + assert _run(["suggest", "--min-saving=0", "-"]) == (0, default_out, b"") + + +def test_one_byte_leaves_out_compact_and_packed_structs() -> None: + assert _run(["suggest", "--min-saving", "1", "-"]) == (0, _FRAME_BLOCK, b"") + + +def test_the_limit_is_inclusive() -> None: + assert _run(["suggest", "--min-saving", "8", "-"]) == (0, _FRAME_BLOCK, b"") + assert _run(["suggest", "--min-saving=9", "-"]) == (0, b"no struct saves at least 9 bytes\n", b"") + + +def test_one_byte_message_uses_the_singular() -> None: + source = b"struct Tight {\n u32 a;\n}\n" + assert _run(["suggest", "--min-saving", "1", "-"], source) == ( + 0, + b"no struct saves at least 1 byte\n", + b"", + ) + + +def test_only_structs_with_a_large_enough_saving_remain() -> None: + status, small_and_big, _ = _run(["suggest", "--min-saving", "1", "-"], _TWO_SAVINGS) + assert status == 0 + assert small_and_big.startswith(b"struct Small: size 8 -> 6 (saves 2)\n") + assert b"struct Big: size 32 -> 24 (saves 8)\n" in small_and_big + status, only_big, _ = _run(["suggest", "--min-saving", "3", "-"], _TWO_SAVINGS) + assert status == 0 + assert only_big.startswith(b"struct Big: size 32 -> 24 (saves 8)\n") + assert b"Small" not in only_big + + +def test_option_works_with_abi_and_after_the_file() -> None: + status, out, stderr = _run(["suggest", "-", "--abi=ilp32", "--min-saving=1"]) + assert (status, stderr) == (0, b"") + assert out == ( + b"struct Frame: size 40 -> 36 (saves 4)\n" + b" struct Packet head;\n" + b" u8 version;\n" + b" struct Wire tail[2];\n" + b" bool ok;\n" + ) + + +def test_the_last_value_wins() -> None: + assert _run(["suggest", "--min-saving", "9", "--min-saving", "8", "-"]) == ( + 0, + _FRAME_BLOCK, + b"", + ) + + +@pytest.mark.parametrize("value", ["1", "999999", "000001", "007"]) +def test_accepts_one_to_six_digits(value: str) -> None: + status, _out, stderr = _run(["suggest", f"--min-saving={value}", "-"]) + assert (status, stderr) == (0, b"") + + +@pytest.mark.parametrize("value", ["", "-1", "+1", "1.0", "x", "1000000", " 1", "1 "]) +def test_rejects_other_values(value: str) -> None: + assert _run(["suggest", f"--min-saving={value}", "-"]) == ( + 2, + b"", + f'structpad: invalid --min-saving "{value}"\n'.encode(), + ) + + +def test_needs_a_value() -> None: + assert _run(["suggest", "--min-saving"]) == (2, b"", b"structpad: --min-saving needs a value\n") + + +@pytest.mark.parametrize("command", ["layout", "check"]) +def test_other_commands_do_not_take_it(command: str) -> None: + for args in ([command, "--min-saving", "1", "-"], [command, "--min-saving=1", "-"]): + assert _run(args) == (2, b"", f"structpad: {command} does not take --min-saving\n".encode()) + + +def test_value_is_escaped_in_the_error() -> None: + assert _run(["suggest", "--min-saving=a\nb", "-"]) == ( + 2, + b"", + b'structpad: invalid --min-saving "a\\nb"\n', + ) diff --git a/tests/test_readme.py b/tests/test_readme.py index 168d0eb..494c215 100644 --- a/tests/test_readme.py +++ b/tests/test_readme.py @@ -56,6 +56,20 @@ def test_readme_documents_ignore_option() -> None: assert required_text in readme +def test_readme_documents_min_saving_option() -> None: + readme = _readme() + + for required_text in ( + "`--min-saving N` is available only with `suggest`", + "structpad suggest --min-saving 8 net.sp", + "no struct saves at least N bytes", + 'invalid --min-saving "-1"', + "layout does not take --min-saving", + "check does not take --min-saving", + ): + assert required_text in readme + + def test_readme_has_worked_input_and_exact_layout_output_as_fenced_blocks() -> None: blocks = _fenced_blocks(_readme())