gemini-code-assist[bot] commented on code in PR #36:
URL: https://github.com/apache/tvm-ffi/pull/36#discussion_r2403504981
##########
python/tvm_ffi/cython/type_info.pxi:
##########
@@ -58,6 +59,82 @@ cdef class FieldSetter:
raise_existing_error()
raise move_from_last_error().py_error()
+_TYPE_SCHEMA_ORIGIN_CONVERTER = {
+ # A few Python-native types
+ "Variant": "Union",
+ "Optional": "Optional",
+ "Tuple": "tuple",
+ "ffi.Function": "Callable",
+ "ffi.Array": "list",
+ "ffi.Map": "dict",
+ "ffi.OpaquePyObject": "Any",
+ "ffi.Object": "Object",
+ "ffi.Tensor": "Tensor",
+ "DLTensor*": "Tensor",
+ # ctype types
+ "void*": "ctypes.c_void_p",
+ # bytes
+ "TVMFFIByteArray*": "bytes",
+ "ffi.SmallBytes": "bytes",
+ "ffi.Bytes": "bytes",
+ # strings
+ "std::string": "str",
+ "const char*": "str",
+ "ffi.SmallStr": "str",
+ "ffi.String": "str",
+}
+
+
[email protected](repr=False, frozen=True)
+class TypeSchema:
+ """Type schema for a TVM FFI type."""
+ origin: str
+ args: tuple[TypeSchema, ...] = ()
+
+ def __post_init__(self):
+ origin = self.origin
+ args = self.args
+ if origin == "Union":
+ assert len(args) >= 2, "Union must have at least two arguments"
+ elif origin == "Optional":
+ assert len(args) == 1, "Optional must have exactly one argument"
+ elif origin == "list":
+ assert len(args) == 1, "list must have exactly one argument"
+ elif origin == "dict":
+ assert len(args) == 2, "dict must have exactly two arguments"
+ elif origin == "tuple":
+ pass # tuple can have arbitrary number of arguments
+
+ def __repr__(self) -> str:
+ if self.origin == "Union":
+ return " | ".join(repr(a) for a in self.args)
+ elif self.origin == "Optional":
+ return repr(self.args[0]) + " | None"
+ elif self.origin == "Callable":
+ if not self.args:
+ return "Callable[..., Any]"
+ else:
+ arg_ret = self.args[0]
+ arg_args = self.args[1:]
+ return f"Callable[[{', '.join(repr(a) for a in arg_args)}],
{repr(arg_ret)}]"
+ elif not self.args:
+ return self.origin
+ else:
+ return f"{self.origin}[{', '.join(repr(a) for a in self.args)}]"
+
+ @staticmethod
+ def from_json_obj(obj: dict[str, Any]) -> "TypeSchema":
+ assert isinstance(obj, dict) and "type" in obj, obj
+ origin = obj["type"]
+ origin = _TYPE_SCHEMA_ORIGIN_CONVERTER.get(origin, origin)
+ args = obj.get("args", ())
+ args = tuple(TypeSchema.from_json_obj(a) for a in args)
+ return TypeSchema(origin, args)
+
+ @staticmethod
+ def from_json_str(s) -> "TypeSchema":
+ return TypeSchema.from_json_obj(json.loads(s))
+
Review Comment:
In `TypeSchema`, `assert` statements are used for validation in
`__post_init__` and `from_json_obj`. These checks will be disabled when Python
is run with the `-O` (optimize) flag, which can lead to unexpected behavior.
For a library, it's more robust to use `if/raise` to ensure validations are
always performed. Additionally, the `s` argument in `from_json_str` could be
type-hinted for better clarity.
```
@dataclasses.dataclass(repr=False, frozen=True)
class TypeSchema:
"""Type schema for a TVM FFI type."""
origin: str
args: tuple[TypeSchema, ...] = ()
def __post_init__(self):
origin = self.origin
args = self.args
if origin == "Union":
if not len(args) >= 2:
raise ValueError("Union must have at least two arguments")
elif origin == "Optional":
if not len(args) == 1:
raise ValueError("Optional must have exactly one argument")
elif origin == "list":
if not len(args) == 1:
raise ValueError("list must have exactly one argument")
elif origin == "dict":
if not len(args) == 2:
raise ValueError("dict must have exactly two arguments")
elif origin == "tuple":
pass # tuple can have arbitrary number of arguments
def __repr__(self) -> str:
if self.origin == "Union":
return " | ".join(repr(a) for a in self.args)
elif self.origin == "Optional":
return repr(self.args[0]) + " | None"
elif self.origin == "Callable":
if not self.args:
return "Callable[..., Any]"
else:
arg_ret = self.args[0]
arg_args = self.args[1:]
return f"Callable[[{', '.join(repr(a) for a in arg_args)}],
{repr(arg_ret)}]"
elif not self.args:
return self.origin
else:
return f"{self.origin}[{', '.join(repr(a) for a in self.args)}]"
@staticmethod
def from_json_obj(obj: dict[str, Any]) -> "TypeSchema":
if not (isinstance(obj, dict) and "type" in obj):
raise TypeError(f"Expected a dict with a 'type' key, but got
{obj!r}")
origin = obj["type"]
origin = _TYPE_SCHEMA_ORIGIN_CONVERTER.get(origin, origin)
args = obj.get("args", ())
args = tuple(TypeSchema.from_json_obj(a) for a in args)
return TypeSchema(origin, args)
@staticmethod
def from_json_str(s: str) -> "TypeSchema":
return TypeSchema.from_json_obj(json.loads(s))
```
##########
src/ffi/extra/json_writer.cc:
##########
@@ -188,41 +178,11 @@ class JSONWriter {
}
void WriteString(const String& value) {
- *out_iter_++ = '"';
- const char* data = value.data();
- const size_t size = value.size();
- for (size_t i = 0; i < size; ++i) {
- switch (data[i]) {
-// handle escape characters per JSON spec(RFC 8259)
-#define HANDLE_ESCAPE_CHAR(pattern, val) \
- case pattern: \
- WriteLiteral(val, std::char_traits<char>::length(val)); \
- break
- HANDLE_ESCAPE_CHAR('\"', "\\\"");
- HANDLE_ESCAPE_CHAR('\\', "\\\\");
- HANDLE_ESCAPE_CHAR('/', "\\/");
- HANDLE_ESCAPE_CHAR('\b', "\\b");
- HANDLE_ESCAPE_CHAR('\f', "\\f");
- HANDLE_ESCAPE_CHAR('\n', "\\n");
- HANDLE_ESCAPE_CHAR('\r', "\\r");
- HANDLE_ESCAPE_CHAR('\t', "\\t");
-#undef HANDLE_ESCAPE_CHAR
- default: {
- uint8_t u8_val = static_cast<uint8_t>(data[i]);
- // this is a control character, print as \uXXXX
- if (u8_val < 0x20 || u8_val == 0x7f) {
- char buffer[8];
- int size = TVM_FFI_SNPRINTF(buffer, sizeof(buffer), "\\u%04x",
- static_cast<int32_t>(data[i]) & 0xff);
- WriteLiteral(buffer, size);
- } else {
- *out_iter_++ = data[i];
- }
- break;
- }
- }
+ String escaped = EscapeString(value);
+ for (size_t i = 0; i < escaped.size(); ++i) {
+ *out_iter_++ = escaped.data()[i];
}
- *out_iter_++ = '"';
+ return;
Review Comment:
The loop to append the escaped string character by character can be made
more efficient and idiomatic by using `std::copy` with the
`back_insert_iterator`.
```c
String escaped = EscapeString(value);
std::copy(escaped.data(), escaped.data() + escaped.size(), out_iter_);
return;
```
--
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]
