gemini-code-assist[bot] commented on code in PR #18415:
URL: https://github.com/apache/tvm/pull/18415#discussion_r2484237720
##########
web/src/runtime.ts:
##########
@@ -1396,7 +1396,18 @@ export class Instance implements Disposable {
});
const recSource = buffer.slice(rec.byteOffset, rec.byteOffset +
rec.nbytes);
// first sync copy to cpu.
- this.ctx.arrayDecodeStorage(cpu_arr, new Uint8Array(recSource),
rec.format, rec.dtype);
+ this.withNewScope(() => {
+ const retValue = this.ctx.arrayDecodeStorage(
+ cpu_arr, new Uint8Array(recSource), rec.format, rec.dtype
+ );
+ if (retValue !== null && retValue instanceof TVMObject) {
+ try {
+ this.detachFromCurrentScope(retValue as any);
+ } catch {
+ // ignore
+ }
Review Comment:
Using a `try...catch` block with an empty `catch` and an `// ignore` comment
is generally considered a code smell as it can silently swallow unexpected
errors, making debugging more difficult. It's better to be more explicit about
the errors being ignored.
If the intention is to ignore the error only when the object is not found in
the current scope (which can happen for tensor views), the `catch` block should
verify the error type or message before ignoring it. This ensures that other
unexpected errors are still propagated.
```typescript
} catch (e) {
// It is fine to ignore when the object is not in the
current scope.
// This can happen if arrayDecodeStorage returns a view of a
tensor,
// which is not tracked by the scope.
if (!(e instanceof Error && e.message.includes("Cannot find
obj"))) {
throw e;
}
}
```
--
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]
