-
Notifications
You must be signed in to change notification settings - Fork 3k
erts: Add guard BIF erlang:is_between/3
#9995
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
base: master
Are you sure you want to change the base?
Conversation
Add guard BIF `erlang:is_between/3` according to EEP-16. This BIF takes in 3 parameters, `Term`, `LowerBound`, and `UpperBound`. It returns `true` if `Term`, `LowerBound`, and `UpperBound` are all integers, and `LowerBound =< Term =< UpperBound`; otherwise, it returns false. Failure: `badarg` if `LowerBound` or `UpperBound` does not evaluate to an integer. Example: ```` 1> is_between(2, 1, 10). true 2> is_between(11, 1, 10). false 3> is_between(1, 1.0, 10.0). ** exception error: bad argument in function is_between/3 called as is_between(1,1.0,10.0) ```` Co-authored-by: Björn Gustavsson <[email protected]> Co-authored-by: John Högberg <[email protected]>
CT Test Results 7 files 640 suites 3h 14m 10s ⏱️ For more details on these failures, see this check. Results for commit a049aef. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
I think the behaviour with regards to types seems somewhat inconsistent. First, I find it a bit odd, it works with just integers, but even then, for bounds it raises an error, if the value is not an integer, while for the middle value it just returns false - I think the behaviour should be consistent. I also wonder, in terms of implementation, if it wouldn't be better, if this was desugared in core to the basic comparisons - the compiler has quite a bit of infrastructure of optimising comparisons that we might be losing here. |
It works only with integers because this has the widest use case. Having different error behaviors for non-integer bounds and non-integer argument makes the BIF convey more meaning in its result. About the implementation: A BIF must have a C implementation so that it can work in scenarios like |
That's not true for something like This actually raises another concern for me - the order of arguments of
It could work with a pure-erlang implementation in the |
I don't have much preference about if we should Well my statement about C implementation is incorrect, but I don't see how a pure Erlang implementation would be more elegant than the current one. If the BIF never throws exception or there's no JIT, desugaring everything would be nice. But now we have the tradeoff between desugaring the BIF in various contexts differently, or calling the BIF through different routes and desugaring when possible. As we don't lose optimizations either way, I prefer the latter because the structure is neater. |
Add guard BIF
erlang:is_between/3
according to EEP-16. This BIF takes in 3 parameters,Term
,LowerBound
, andUpperBound
. It returnstrue
ifTerm
,LowerBound
, andUpperBound
are all integers, andLowerBound =< Term =< UpperBound
; otherwise, it returns false.Failure:
badarg
ifLowerBound
orUpperBound
does not evaluate to an integer.Example:
The PR is ready for review, but it can't be merged until it's approved by an OTB meeting.