Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Odd difference between ErlNifTermType and TermType returned by Term.get_type() #380

Closed
Qqwy opened this issue Sep 8, 2021 · 4 comments
Closed

Comments

@Qqwy
Copy link
Contributor

Qqwy commented Sep 8, 2021

enif_get_type returns an enum value depending on the type of value a particular term is.

Rustler currently uses Term.get_type(), which returns a TermType.

There is a difference in the types returned by these two approaches.
Under the hood, get_type is implemented as a bunch of is_* checks.

However, there are two downsides with this approach:

  1. They are slower;
  2. They do not disambiguate between integers and floats.

rustler_sys does seem to know about the ErlNifTermType, but it doesn't seem to be used anywhere else in the code right now.

What would be the best way to improve this situation? I would be willing to contribute with a PR or two if someone can tell me which approach to take.

@evnu
Copy link
Member

evnu commented Sep 8, 2021

Note that enif_get_type is rather recent (OTP 22). Rustler currently supports OTP 20.3 as well (at least according to the github actions). To implement that in a backwards-compatible way, Term.get_type() could call into a feature-gated helper. For OTP >= 22, this would call into enif_get_type and match on the resulting enum, and for OTP < 22, the current logic would be used. See Term.map_from_arrays() for an example on functions feature-gated to the NIF version.

The feature-gate mechanism uses build.rs to determine the NIF version. The version check can be overridden with RUSTLER_NIF_VERSION as well.

They do not disambiguate between integers and floats.

As long as we need to maintain backwards compatibility, this would still be the case.

@evnu
Copy link
Member

evnu commented Sep 8, 2021

TermType and ErlNifTermType don't match exactly. For example, TermType has Number, while ErlNifTermType does not. Maybe we should have another function then to implement enif_get_type? It is unfortunate that Term.get_type() is already taken. So maybe we should not implement this for OTP < 22 at all, but instead disambiguate between the two?

@evnu
Copy link
Member

evnu commented Aug 17, 2023

See #538, which tackles this problem.

@filmor
Copy link
Member

filmor commented Nov 14, 2023

#538 has been merged and released, closing this.

@filmor filmor closed this as completed Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants