guan404ming commented on PR #399: URL: https://github.com/apache/tvm-ffi/pull/399#issuecomment-3823923434
> In comparison, the enum-based error would be more efficient, but I don't think it's a blocker. To support this, we may consider supporting errno in the Error class in the future. Another consideration is whether we should expose ffi::expected to Python. I feel it is nice to have in the future. For this PR, it may not be required to simplify the scope. Thanks for the detailed review and the XGrammar comparison! The enum-based error and Python exposure are both good ideas worth exploring. I think we can keep them as follow-up items to keep this PR focused on the core Expected<T> with implicit conversion support. > Currently, in XGrammar's result library, [ResultOk(...) and ResultErr(...)](https://github.com/Ubospica/xgrammar/blob/15dc61d02ed24820215ca1d491d6a07ba32fff50/cpp/support/utils.h#L124-L212) are used to return success or error values, and I think it is easy to use. The current implementation removes them since implicit conversion makes them redundant. But given that XGrammar uses a similar pattern (ResultOk/ResultErr), I want to confirm whether we prefer to keep both options available for users who favor the explicit style. > I think to add implicit converting constructors would be ideal to handle this feat: Just updated with implicit conversion as planned please take another look thanks! -- 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]
