gemini-code-assist[bot] commented on code in PR #309: URL: https://github.com/apache/tvm-ffi/pull/309#discussion_r2587453812
########## python/tvm_ffi/utils/kwargs_wrapper.py: ########## @@ -0,0 +1,357 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Utilities for creating high-performance keyword argument wrapper functions. + +This module provides tools for wrapping positional-only callables with +keyword argument support using code generation techniques. +""" + +from __future__ import annotations + +import functools +import inspect +from typing import Any, Callable + +# Sentinel object for missing arguments +MISSING = object() + + +def _validate_argument_names(names: list[str], arg_type: str) -> None: + """Validate that argument names are valid Python identifiers. + + Parameters + ---------- + names + List of argument names to validate. + arg_type + Description of the argument type (e.g., "Argument", "Keyword-only argument"). + + """ + for name in names: + if not isinstance(name, str): + raise TypeError( + f"{arg_type} name must be a string, got {type(name).__name__}: {name!r}" + ) + if not name.isidentifier(): + raise ValueError( + f"Invalid {arg_type.lower()} name: {name!r} is not a valid Python identifier" + ) + + +def _validate_wrapper_args( + args_names: list[str], + args_defaults: tuple, + kwargsonly_names: list[str], + kwargsonly_defaults: dict[str, Any], + reserved_names: set[str], +) -> None: + """Validate all input arguments for make_kwargs_wrapper. + + Parameters + ---------- + args_names + List of positional argument names. + args_defaults + Tuple of default values for positional arguments. + kwargsonly_names + List of keyword-only argument names. + kwargsonly_defaults + Dictionary of default values for keyword-only arguments. + reserved_names + Set of reserved internal names that cannot be used as argument names. + + """ + # Validate args_names are valid Python identifiers + _validate_argument_names(args_names, "Argument") + + # Validate args_defaults is a tuple + if not isinstance(args_defaults, tuple): + raise TypeError(f"args_defaults must be a tuple, got {type(args_defaults).__name__}") + + # Validate args_defaults length doesn't exceed args_names length + if len(args_defaults) > len(args_names): + raise ValueError( + f"args_defaults has {len(args_defaults)} values but only " + f"{len(args_names)} positional arguments" + ) + + # Validate kwargsonly_names are valid identifiers + _validate_argument_names(kwargsonly_names, "Keyword-only argument") Review Comment:  The validation logic is missing a check for duplicate argument names within `args_names` and `kwargsonly_names`. Since Python function definitions do not allow duplicate arguments, providing a list with duplicates will cause `exec()` to fail with a generic `SyntaxError`. It would be more user-friendly to add an explicit check at the beginning of this function and raise a `ValueError` with a list of the duplicate names. ```python # Check for duplicate argument names if len(set(args_names)) != len(args_names): seen = set() dupes = {x for x in args_names if x in seen or seen.add(x)} raise ValueError(f"Duplicate positional argument names found: {sorted(list(dupes))}") if len(set(kwargsonly_names)) != len(kwargsonly_names): seen = set() dupes = {x for x in kwargsonly_names if x in seen or seen.add(x)} raise ValueError(f"Duplicate keyword-only argument names found: {sorted(list(dupes))}") # Validate args_names are valid Python identifiers _validate_argument_names(args_names, "Argument") # Validate args_defaults is a tuple if not isinstance(args_defaults, tuple): raise TypeError(f"args_defaults must be a tuple, got {type(args_defaults).__name__}") # Validate args_defaults length doesn't exceed args_names length if len(args_defaults) > len(args_names): raise ValueError( f"args_defaults has {len(args_defaults)} values but only " f"{len(args_names)} positional arguments" ) # Validate kwargsonly_names are valid identifiers _validate_argument_names(kwargsonly_names, "Keyword-only argument") ``` -- 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]
