This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 8fd93039 [STUBGEN][RUST] Update the check for generated bindings (#861)
8fd93039 is described below
commit 8fd93039ff332d6262d1f6a47f626b6f91a84080
Author: Linzhang Li <[email protected]>
AuthorDate: Thu Oct 22 20:48:55 2026 -0400
[STUBGEN][RUST] Update the check for generated bindings (#861)
## Summary
Make `tvm-ffi-stubgen --target rust --check` report a file as stale only
when
a generated binding would change, not when a run would only respell it.
Until now `--check` was `--dry-run` plus a byte comparison of the whole
file.
A tvm-ffi release that only changes how the generator spells its output
(the
`use` paths, e.g. `super::` to `crate::`, the wrapping of a call, the
wording
of a doc comment) made every downstream tree fail the check, though the
bindings were the same.
With `--target rust`, a file is now stale when one of these would
change:
- a struct field, its type, or the size / alignment assertion
- the type key, `#[type_final]`, the parent, or an upcast
- an accessor or allocator signature
- a builtin stand-in in the import section (it carries `TYPE_DEPTH`)
- the set of `object/` blocks (a `prefix` roll-out to add one, an
unregistered type to remove)
and not when only these would:
- `use` lines: their paths, their order, one more or one fewer
- comment lines
- line wrapping and trailing commas
- the module a path is spelled from (`crate::ir::Expr`,
`super::super::ir::Expr` and `Expr` are the same)
## Changes
- `Generator.check_view(code_blocks)` returns what `--check` compares.
The
pipeline takes it for each file before the run edits any block (before
the
`prefix` roll-out), and a file the run changes is stale when its view
differs afterwards.
- `PythonGenerator.check_view` is the whole text, so the Python target
behaves
as before.
- `RustGenerator.check_view` maps each `object/` block and the import
section
to its tokens from `check_tokens`, which drops `use` and comment lines,
module qualifiers, and a comma before a closing bracket. An import
section
of `use` lines alone has no entry, so the one a `prefix` roll-out adds
does
not count.
- `docs/packaging/stubgen.rst` and `--help` describe the rule.
## Compatibility
- Python target: unchanged.
- Rust target: `--check` passes on trees that differ from the generated
text
only in spelling, and still fails on every change listed above. A run
without `--check` still rewrites the file to the exact generated text.
- Not reported: a field whose type moves to a type of the same name in
another
module (`s_tir::FunctionFrame` to `tirx::FunctionFrame`), since that
changes
only a module path.
---
docs/packaging/stubgen.rst | 5 ++
python/tvm_ffi/stub/cli.py | 9 ++-
python/tvm_ffi/stub/generator.py | 4 ++
python/tvm_ffi/stub/python_generator/generator.py | 4 ++
python/tvm_ffi/stub/rust_generator/generator.py | 16 ++++-
python/tvm_ffi/stub/rust_generator/utils.py | 24 +++++++
tests/python/test_stubgen_rust.py | 86 +++++++++++++++++++++++
7 files changed, 145 insertions(+), 3 deletions(-)
diff --git a/docs/packaging/stubgen.rst b/docs/packaging/stubgen.rst
index 19a64eea..f9271f28 100644
--- a/docs/packaging/stubgen.rst
+++ b/docs/packaging/stubgen.rst
@@ -250,6 +250,11 @@ All three are required together. When omitted, the tool
operates in directive-on
date and exit with status 1 if there is any (2 if a file failed to
process), so CI
can reject a stale tree. Cannot be combined with ``--init-*``.
+ With ``--target rust``, a file is stale only when a generated definition
changes:
+ a type, field, accessor, allocator, upcast or layout assertion, or an
``object/``
+ block to add or remove. A run that would only rewrite ``use`` lines,
comments,
+ line wrapping or the module path a type is spelled from leaves the file up
to date.
+
``--imports``
Additional Python modules to import before generation (semicolon-separated).
diff --git a/python/tvm_ffi/stub/cli.py b/python/tvm_ffi/stub/cli.py
index ca806383..eb3381c6 100644
--- a/python/tvm_ffi/stub/cli.py
+++ b/python/tvm_ffi/stub/cli.py
@@ -98,6 +98,9 @@ def __main__() -> int:
generator=generator,
)
+ # `--check` compares the generator's view of each file before the run
edits any block.
+ before = {file.path: generator.check_view(file.code_blocks) for file in
files if opt.check}
+
# Stage 2b. Add the object blocks a `tvm-ffi-stubgen(prefix)` file asks
for. This runs
# after `--init`, which rewrites files on disk and reloads them.
failed += _roll_out_prefixes(files, generator)
@@ -125,7 +128,7 @@ def __main__() -> int:
f'{C.TERM_RED}[Failed] File "{file.path}":
{traceback.format_exc()}{C.TERM_RESET}'
)
continue
- if changed and opt.check:
+ if changed and opt.check and generator.check_view(file.code_blocks) !=
before[file.path]:
stale += 1
print(f"{C.TERM_YELLOW}[Stale] {file.path}{C.TERM_RESET}")
@@ -537,7 +540,9 @@ def _parse_args() -> Options:
action="store_true",
help=(
"Don't write changes; report every file whose stub blocks are out
of date "
- "and exit with status 1 if there is any. Cannot be combined with
--init-*."
+ "and exit with status 1 if there is any. For --target rust, only
the generated "
+ "definitions count, not `use` lines, comments, layout or module
paths. "
+ "Cannot be combined with --init-*."
),
)
parser.add_argument(
diff --git a/python/tvm_ffi/stub/generator.py b/python/tvm_ffi/stub/generator.py
index 7e4055d5..3a8c6885 100644
--- a/python/tvm_ffi/stub/generator.py
+++ b/python/tvm_ffi/stub/generator.py
@@ -122,6 +122,10 @@ class Generator(Protocol):
"""
...
+ def check_view(self, code_blocks: list[CodeBlock]) -> object:
+ """Return what ``--check`` compares: a file is stale when a run
changes it."""
+ ...
+
# --- per-block generation (mutates `code.lines`) ------------------------
def generate_global_funcs_block(
diff --git a/python/tvm_ffi/stub/python_generator/generator.py
b/python/tvm_ffi/stub/python_generator/generator.py
index 04dd6670..40a47bfa 100644
--- a/python/tvm_ffi/stub/python_generator/generator.py
+++ b/python/tvm_ffi/stub/python_generator/generator.py
@@ -79,6 +79,10 @@ class PythonGenerator:
"""Whether the ``tvm_ffi`` package binds ``type_key`` itself."""
return type_key in C.BUILTIN_TYPE_KEYS
+ def check_view(self, code_blocks: list[CodeBlock]) -> tuple[str, ...]:
+ """Return the whole text: any change to a stub block makes the file
stale."""
+ return tuple(line for code in code_blocks for line in code.lines)
+
# --- per-block generation (mutates `code.lines`) ------------------------
def generate_global_funcs_block(
diff --git a/python/tvm_ffi/stub/rust_generator/generator.py
b/python/tvm_ffi/stub/rust_generator/generator.py
index 27f16567..c7d0a92c 100644
--- a/python/tvm_ffi/stub/rust_generator/generator.py
+++ b/python/tvm_ffi/stub/rust_generator/generator.py
@@ -33,7 +33,7 @@ from .codegen import (
generate_rust_import_section,
generate_rust_object,
)
-from .utils import RustImports, RustUse, is_crate_type_key, rust_module_path
+from .utils import RustImports, RustUse, check_tokens, is_crate_type_key,
rust_module_path
if TYPE_CHECKING:
from pathlib import Path
@@ -98,6 +98,20 @@ class RustGenerator:
"""Every ``ffi.*`` key is bound by the crate (the rule ``codegen``
resolves types with)."""
return is_crate_type_key(type_key)
+ def check_view(self, code_blocks: list[CodeBlock]) -> dict[tuple[str,
object], tuple[str, ...]]:
+ """Return what the ``object/`` blocks and the import section define,
not their spelling.
+
+ A run that only changes ``use`` lines, comments, layout or the module
a path is
+ spelled from (see :func:`check_tokens`) leaves the file up to date.
+ """
+ view: dict[tuple[str, object], tuple[str, ...]] = {}
+ for code in code_blocks:
+ if code.kind in ("object", "import-section"):
+ tokens = check_tokens(code.lines[1:-1])
+ if tokens: # an import section of `use` lines alone is no
definition
+ view[code.kind, code.param] = tokens
+ return view
+
# --- per-block generation
-------------------------------------------------
def generate_global_funcs_block(
diff --git a/python/tvm_ffi/stub/rust_generator/utils.py
b/python/tvm_ffi/stub/rust_generator/utils.py
index 8bf2cce3..375b217e 100644
--- a/python/tvm_ffi/stub/rust_generator/utils.py
+++ b/python/tvm_ffi/stub/rust_generator/utils.py
@@ -20,6 +20,7 @@ from __future__ import annotations
import dataclasses
import functools
+import re
from pathlib import Path
from typing import TYPE_CHECKING, Callable
@@ -27,6 +28,8 @@ from . import consts as C
from .directives import Directives
if TYPE_CHECKING:
+ from collections.abc import Iterable
+
from tvm_ffi.core import TypeSchema
@@ -169,6 +172,27 @@ def _generic(base: str | None, *params: str | None) -> str
| None:
return f"{base}<{', '.join(p for p in params if p is not None)}>"
+#: The module qualifier of a path (``crate::ir::``, ``super::super::``,
``::core::mem::``).
+_MODULE_QUALIFIER =
re.compile(r"(?<![\w:])(?:::)?(?:[a-z_][a-z0-9_]*::)+(?=[A-Za-z_])")
+
+
+def check_tokens(lines: Iterable[str]) -> tuple[str, ...]:
+ """Return the tokens of generated Rust ``lines`` that ``--check`` compares.
+
+ ``use`` lines, comment lines and module qualifiers are dropped, and so is a
+ trailing comma, so neither the import paths nor how rustfmt wraps a call
count.
+ """
+ text = " ".join(
+ line for line in (raw.strip() for raw in lines) if not
line.startswith(("use ", "//"))
+ )
+ tokens = re.findall(r"\w+|\S", _MODULE_QUALIFIER.sub("", text))
+ return tuple(
+ token
+ for token, after in zip(tokens, [*tokens[1:], ""])
+ if not (token == "," and after in (")", "]", "}"))
+ )
+
+
def rust_ident(name: str) -> str:
"""Spell a reflected field name in Rust.
diff --git a/tests/python/test_stubgen_rust.py
b/tests/python/test_stubgen_rust.py
index 5d4c9a97..01375e77 100644
--- a/tests/python/test_stubgen_rust.py
+++ b/tests/python/test_stubgen_rust.py
@@ -44,6 +44,7 @@ from tvm_ffi.stub.rust_generator.directives import
Directives, EnumSpec
from tvm_ffi.stub.rust_generator.utils import (
RustImports,
RustUse,
+ check_tokens,
render_rust_type,
rust_ident,
rust_module_path,
@@ -266,6 +267,32 @@ def test_rust_ident() -> None:
assert rust_ident("data") == "data" # the wrapper slot is `base`, so
`data` is free
+def test_check_tokens_ignores_spelling() -> None:
+ generated = [
+ "use super::super::ir::ExprObj;",
+ "/// Complete: every field is mirrored.",
+ "#[derive(tvm_ffi::derive::Object)]",
+ "pub struct AddObj { base: ExprObj, pub a: i64, pub b: i64 }",
+ "const _: () = assert!(::core::mem::size_of::<AddObj>() == 40);",
+ "tvm_ffi::impl_object_upcast!(Add => super::super::ir::Expr);",
+ ]
+ respelled = [
+ "use crate::ir::Expr;",
+ "#[derive(Object)]",
+ "pub struct AddObj {",
+ " base: crate::ir::ExprObj,",
+ " pub a: i64,",
+ " pub b: i64,",
+ "}",
+ "const _: () = assert!(core::mem::size_of::<AddObj>() == 40);",
+ "impl_object_upcast!(Add => Expr);",
+ ]
+ assert check_tokens(generated) == check_tokens(respelled)
+ for drift in [("a: i64", "a: i32"), ("40", "48"), ("ExprObj,",
"StmtObj,"), ("Expr)", "Stmt)")]:
+ changed = [line.replace(*drift) for line in respelled]
+ assert check_tokens(generated) != check_tokens(changed), drift
+
+
# ---------------------------------------------------------------------------
# Directives
# ---------------------------------------------------------------------------
@@ -1443,6 +1470,65 @@ def test_cli_check_reports_stale_files(
assert stub_cli.__main__() == 0
+def test_cli_check_ignores_how_a_binding_is_spelled(
+ tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys:
pytest.CaptureFixture[str]
+) -> None:
+ root = _crate(tmp_path)
+ monkeypatch.setattr(
+ "sys.argv",
+ [
+ "tvm-ffi-stubgen",
+ "--target",
+ "rust",
+ "--init-pypkg",
+ "demo",
+ "--init-lib",
+ "demo_shared",
+ "--init-prefix",
+ "testing.",
+ str(root),
+ ],
+ )
+ assert stub_cli.__main__() == 0
+ mod_rs = root / "testing" / "mod.rs"
+ fresh = mod_rs.read_text(encoding="utf-8")
+ # Imports, paths, comments and wrapping differ from what a run writes; the
bindings do not.
+ respelled = fresh
+ for old, new in [
+ ("use tvm_ffi::String;\n", ""),
+ ("use std::ops::Deref;\n", "use std::ops::Deref;\nuse
crate::testing::TestCompare;\n"),
+ (" base: Object,\n", " base: tvm_ffi::Object,\n"),
+ ("/// Complete: ", "/// Mirrored: "),
+ (
+ " pub(crate) fn new(\n"
+ " p_required: i64,\n"
+ " p_default: i64,\n"
+ " k_required: i64,\n"
+ " k_default: i64,\n"
+ " hidden: i64,\n"
+ " ) -> Self {\n",
+ " pub(crate) fn new(p_required: i64, p_default: i64,
k_required: i64, "
+ "k_default: i64, hidden: i64) -> Self {\n",
+ ),
+ ]:
+ assert old in respelled, old
+ respelled = respelled.replace(old, new)
+ mod_rs.write_text(respelled, encoding="utf-8")
+ check = ["tvm-ffi-stubgen", "--target", "rust", "--check", str(root)]
+ monkeypatch.setattr("sys.argv", check)
+ assert stub_cli.__main__() == 0
+ assert "[Stale]" not in capsys.readouterr().out
+ assert mod_rs.read_text(encoding="utf-8") == respelled
+ # A layout that no longer matches the registry is stale.
+ drifted = respelled.replace(
+ "size_of::<TestCompareObj>() == 56", "size_of::<TestCompareObj>() ==
48"
+ )
+ assert drifted != respelled
+ mod_rs.write_text(drifted, encoding="utf-8")
+ assert stub_cli.__main__() == 1
+ assert f"[Stale] {mod_rs}" in capsys.readouterr().out
+
+
def test_cli_failed_file_exits_2(tmp_path: Path, monkeypatch:
pytest.MonkeyPatch) -> None:
root = _crate(tmp_path)
(root / "mod.rs").write_text(
