ykjit / yk

yk packages
https://ykjit.github.io/yk/
Other
28 stars 7 forks source link

De-`TiVec`-ify #1257

Closed ltratt closed 1 month ago

ltratt commented 1 month ago

TiVec has the unfortunate property that it requires us to implement impl From<usize> for *Idx, which means we have a method which silently takes an arbitrary usize and tries to squeeze it into a u16. Even though the method has a big warning on it, the nature of trait method resolution means that it's unlikely we'll notice that. The From is thus an unfortunate footgun.

On reflection, I think we don't really need TiVec inside jit_ir/mod.rs -- its main utility is to force users of the JIT IR to be careful with their use of types, which happens naturally because they're forced to use methods which take in, and return, *Idx types. This commit thus de-TiVecifies jit_ir/mod.rs.

vext01 commented 1 month ago

I suppose ideally we'd have a TiVec whose index type (and thus max length too) is parameterised. The issue stems from the assumption that vector indices are usize.

This seems OK to me. We just let the API do the external type safety, and internally you have to just be careful.

ltratt commented 1 month ago

I suppose ideally we'd have a TiVec whose index type (and thus max length too) is parameterised. The issue stems from the assumption that vector indices are usize.

It's a surprisingly hard problem, in essence because trait implementations are sort-of global. I'm not sure that one can get the effect one would ideally like.

ltratt commented 1 month ago

Oops! Force pushed cargo fmt update.