Skip to content

Add is_inf and is_nan functions to builtin:: #22059

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

Open
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented Mar 1, 2024

As it is not expected that these two functions are likely to be called very often, I didn't bother making inlined-opcode variants of them as per e.g. is_bool(). Instead these are just regular XSUBs like trim() is.

Testing-wise, I'm a little unsure about the use of 1 * (inf - inf) to get a calculated NaN value, but I really couldn't think of anything else that actually works. Is it portable enough?

@leonerd leonerd force-pushed the builtin-is_inf-is_nan branch from 5d16681 to e2ead2e Compare March 1, 2024 23:09
As it is not expected that these two functions are likely to be called
very often, I didn't bother making inlined-opcode variants of them as
per e.g. is_bool(). Instead these are just regular XSUBs like trim() is.
@leonerd leonerd force-pushed the builtin-is_inf-is_nan branch from e2ead2e to 87623dc Compare March 2, 2024 11:47
if(items != 1)
croak_xs_usage(cv, "val");
SvGETMAGIC(val);
if(SvNOK(val) && Perl_isinf(SvNV(val)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NOK check seems non-perlish to me, it also breaks overloads:

tony@venus:.../git/perl6$ ./perl -Ilib ../22059.pl 
Built-in function 'builtin::inf' is experimental at ../22059.pl line 6.
Built-in function 'builtin::is_inf' is experimental at ../22059.pl line 11.
Built-in function 'builtin::is_inf' is experimental at ../22059.pl line 13.

1

1
tony@venus:.../git/perl6$ cat ../22059.pl
use v5.38;
use builtin qw(is_inf inf);
use POSIX();

package Foo {
  use overload '0+' => sub { inf };
}

my $x = bless {}, "Foo";

say is_inf($x);
say POSIX::isinf($x);
say is_inf("Inf");
say POSIX::isinf("Inf");

@tonycoz
Copy link
Contributor

tonycoz commented Mar 3, 2024

Another issue with this (and ceil(), floor() for that matter), is it doesn't support overloads in the same way other numeric functions do, e.g.:

use overload sin => sub { ... };

It's less of an issue if the 0+ overload returns a Inf/Nan as needed (which this code also doesn't support), but I can see it as an issue for ceil/floor with bignum-ish overloads.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 13, 2024

Another issue with this (and ceil(), floor() for that matter), is it doesn't support overloads in the same way other numeric functions do, e.g.:

use overload sin => sub { ... };

It's less of an issue if the 0+ overload returns a Inf/Nan as needed (which this code also doesn't support), but I can see it as an issue for ceil/floor with bignum-ish overloads.

@leonerd, @tonycoz has noted some significant problems with this pull request. Do you wish to pursue it further? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants