gemini-code-assist[bot] commented on code in PR #259:
URL: https://github.com/apache/tvm-ffi/pull/259#discussion_r2525259490
##########
python/tvm_ffi/stub/stubgen.py:
##########
@@ -434,7 +99,42 @@ def __main__() -> int:
in the given files or directories. See the module docstring for an
overview and examples of the block syntax.
"""
-
+ opt = _parse_args()
+ dlls = [ctypes.CDLL(lib) for lib in opt.dlls]
+ global_funcs = _collect_global_funcs()
+ files: list[FileInfo] = collect_files([Path(f) for f in opt.files])
+
+ # Stage 1: Process `tvm-ffi-stubgen(ty-map)`
+ ty_map: dict[str, str] = _collect_ty_maps(files, opt)
+ # Stage 2: Process
+ # - `tvm-ffi-stubgen(begin): global/...`
+ # - `tvm-ffi-stubgen(begin): object/...`
+ for file in files:
+ if opt.verbose:
+ print(f"{C.TERM_CYAN}[File] {file.path}{C.TERM_RESET}")
+ ty_used: set[str] = set()
+ ty_on_file: set[str] = set()
+ fn_ty_map_fn = _fn_ty_map(ty_map, ty_used)
+ # Stage 2.1. Process `tvm-ffi-stubgen(begin): global/...`
+ for code in file.code_blocks:
+ if code.kind == "global":
+ G.generate_global_funcs(code, global_funcs, fn_ty_map_fn, opt)
+ # Stage 2.2. Process `tvm-ffi-stubgen(begin): object/...`
+ for code in file.code_blocks:
+ if code.kind == "object":
+ G.generate_object(code, fn_ty_map_fn, opt)
+ ty_on_file.add(ty_map.get(code.param, code.param))
+ # Stage 2.3. Add imports for used types.
+ for code in file.code_blocks:
+ if code.kind == "import":
+ G.generate_imports(code, ty_used - ty_on_file, opt)
+ break # Only one import block per file is supported for now.
+ file.update(show_diff=opt.verbose, dry_run=opt.dry_run)
+ del dlls
+ return 0
Review Comment:
The main logic has a couple of regressions compared to the previous
implementation:
1. **Error Handling:** The file processing loop lacks a `try...except`
block. If an error occurs during code generation for a single file, the entire
`stubgen` process will crash. The previous version handled errors on a per-file
basis and continued with other files. This is a regression in robustness.
2. **User Feedback:** The tool no longer prints status messages like
`[Updated]` or `[Unchanged]` for each file, making it less clear what the tool
has done.
Please reintroduce the per-file error handling and status messages to
improve robustness and user experience.
```python
opt = _parse_args()
dlls = [ctypes.CDLL(lib) for lib in opt.dlls]
global_funcs = _collect_global_funcs()
files: list[FileInfo] = collect_files([Path(f) for f in opt.files])
# Stage 1: Process `tvm-ffi-stubgen(ty-map)`
ty_map: dict[str, str] = _collect_ty_maps(files, opt)
rc = 0
# Stage 2: Process
# - `tvm-ffi-stubgen(begin): global/...`
# - `tvm-ffi-stubgen(begin): object/...`
for file in files:
try:
if opt.verbose:
print(f"{C.TERM_CYAN}[File] {file.path}{C.TERM_RESET}")
ty_used: set[str] = set()
ty_on_file: set[str] = set()
fn_ty_map_fn = _fn_ty_map(ty_map, ty_used)
# Stage 2.1. Process `tvm-ffi-stubgen(begin): global/...`
for code in file.code_blocks:
if code.kind == "global":
G.generate_global_funcs(code, global_funcs,
fn_ty_map_fn, opt)
# Stage 2.2. Process `tvm-ffi-stubgen(begin): object/...`
for code in file.code_blocks:
if code.kind == "object":
G.generate_object(code, fn_ty_map_fn, opt)
ty_on_file.add(ty_map.get(code.param, code.param))
# Stage 2.3. Add imports for used types.
for code in file.code_blocks:
if code.kind == "import":
G.generate_imports(code, ty_used - ty_on_file, opt)
break # Only one import block per file is supported for
now.
if file.update(show_diff=opt.verbose, dry_run=opt.dry_run):
if not opt.dry_run and not opt.verbose:
print(f"{C.TERM_GREEN}[Updated]
{file.path}{C.TERM_RESET}")
elif opt.verbose:
print(f"{C.TERM_BOLD}[Unchanged] {file.path}{C.TERM_RESET}")
except Exception:
import traceback
print(f'{C.TERM_RED}[Failed] File
"{file.path}"\n{traceback.format_exc()}{C.TERM_RESET}')
rc = 1
del dlls
return rc
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
