gemini-code-assist[bot] commented on code in PR #18385:
URL: https://github.com/apache/tvm/pull/18385#discussion_r2450394478


##########
web/src/ctypes.ts:
##########
@@ -51,6 +51,22 @@ export const enum SizeOf {
  * We are keeping the same style as C API here.
  */
 export const enum TypeIndex {
+  /*
+   * \brief The root type of all FFI objects.
+   *
+   * We include it so TypeIndex captures all possible runtime values.
+   * `kTVMFFIAny` code will never appear in Any::type_index.
+   * However, it may appear in field annotations during reflection.
+   */
+  kTVMFFIAny = -1,
+  // [Section] On-stack POD and special types: [0, kTVMFFIStaticObjectBegin)
+  // N.B. `kTVMFFIRawStr` is a string backed by a `\0`-terminated char array,
+  // which is not owned by TVMFFIAny. It is required that the following
+  // invariant holds:
+  // - `Any::type_index` is never `kTVMFFIRawStr`
+  // - `AnyView::type_index` can be `kTVMFFIRawStr`
+  //
+  /*! \brief None/nullptr value */

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The comment `/*! \brief None/nullptr value */` seems redundant given the 
following line `kTVMFFINone = 0,`. Consider removing the comment or elaborating 
on it if it provides additional context beyond what the code already expresses.
   
   It's generally good to avoid comments that simply restate the code, as they 
can become outdated and misleading if the code changes without the comment 
being updated accordingly.



##########
web/src/runtime.ts:
##########
@@ -193,7 +193,7 @@ class RuntimeContext implements Disposable {
     this.moduleImport = getGlobalFunc("ffi.ModuleImportModule");
     this.tensorEmpty = getGlobalFunc("runtime.TVMTensorAllocWithScope");
     this.tensorCopyFromTo = getGlobalFunc("runtime.TVMTensorCopyFromTo");
-    this.tensorCopyFromJSBytes = 
getGlobalFunc("tvmjs.runtime.NDTensorCopyFromBytes");

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   To maintain consistency, consider renaming `tensorCopyFromJSBytes` to 
`tensorCopyFromBytes` to align with the naming convention used for 
`tensorCopyToJSBytes`.
   
   ```suggestion
       this.tensorCopyFromTo = getGlobalFunc("runtime.TVMTensorCopyFromTo");
       this.tensorCopyFromBytes = 
getGlobalFunc("tvmjs.runtime.TensorCopyFromBytes");
       this.tensorCopyToJSBytes = 
getGlobalFunc("tvmjs.runtime.TensorCopyToBytes");
   ```



-- 
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]

Reply via email to