oraluben commented on PR #592: URL: https://github.com/apache/tvm-ffi/pull/592#issuecomment-4485645084
The current fix is not ideal for two reasons: 1. **Unconditionally clears** cause_chain and extra_context — this would incorrectly discard legitimately set values from `Error(kind, msg, bt, cause_chain, extra_context)` (the 5-arg constructor). 2. **Scattered across two call sites** (SetRaised + SetRaisedByCstr) rather than a single centralized point. A more proper fix could be one of: - Zero the memory in the allocator: `std::memset(data, 0, sizeof(T))` in `SimpleObjAllocator::Handler::New` after allocation and before placement new. This guarantees all POD fields are zeroed regardless of constructor, and is the most systematic approach. - Or use member initializer lists in `ErrorObj` to avoid relying on constructor body execution. @tqchen @junrushao would appreciate your thoughts on the right approach here. -- 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]
