[GitHub] [incubator-tvm] jroesch commented on pull request #6545: [Rust] change Context.device_id from usize to c_int (#6528)

[GitBox <gi...@apache.org>]

jroesch commented on pull request #6545:
URL: https://github.com/apache/incubator-tvm/pull/6545#issuecomment-699462827


   @intheworld we have repaired the ResNet example on #6563 to the best of my knowledge does no longer exhibits this issue. The previous iteration of the Rust bindings pre-object system used a large amount of unsafe code which Max and I have tried to audit and remove. 
   
   Can you provide a reproducible test case to accompany the change, even if we do so I don't believe c_int is the right type to put in the Rust interface as the issue you described only sounds like an issue between signed and unsigned values. 
   
   In general it is bad form in Rust to expose C types in safe Rust interfaces vs. native Rust types, given we convert the Rust Context to DLContext when passing across the boundaries I don't clearly see why this should be an issue?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org