Fixes re List::Util::uniqint

[sisyphus]

This PR addresses all of the uniqint issues that I've come across to date.
I don't guarantee that there are no more issues yet to be uncovered.

A good sample of the problems presented by the current implementation of uniqint (in Scalar-List-Utils-1.55) can be seen by running this PR's modified t/uniq.t against Scalar-List-Utils-1.55
Also tidied up a test in t/uniqnum.t.

Cheers,
Rob

[zhmylove]

@sisyphus I see a lot of improvements for uniqint here.
May I ask you to fix also a SIGSEGV in it?

I'd suggest to add also these tests:

use List::Util qw( uniqint );
use Math::BigInt;
my $obj;
$obj = Math::BigInt->new(5);
is_deeply( [ uniqint $obj, $obj ],
           [ $obj ],
           'uniqint compares Math::BigInt objects' );

$obj = bless {}, __PACKAGE__;
is_deeply( [ uniqint $obj, $obj ],
           [ $obj ],
           'uniqint compares non-numerical objects' );

Segmentation violation exists both in current and your code.

@leonerd am I right that we don't want any segfaults from such a code? UPD: created another PR with fix: #137

[sisyphus]

I think this should be treated as a separate issue, as it's not related to anything being done by this PR (AFAICS).
Could you do that please ?

You can work around it by using $obj in numeric context prior to passing it to uniqint().
Or you can just interpolate $obj prior to passing it to uniqint().
That allows uniqint() to run and the test to pass.
Here's a demo that works (I guess) as expected:

use strict;
use warnings;
use List::Util qw(uniqint);
use Math::BigInt;
use Test::More;

my $obj = bless {}, __PACKAGE__;

$obj < 0; # Allows uniqint() to run.
# "$obj";   # Also allows uniqint() to run.

is_deeply( [ uniqint $obj, $obj ],
           [ $obj ],
           'uniqint compares non-numerical objects' );

done_testing();

Of course, it also outputs a "Useless use of ..." warning, even though we have just proven that our "use" was not "useless" ;-)

A proper fix would address the issue inside ListUtil.xs such that the segfault is avoided.
I had a go at getting that to work, but couldn't quickly find the correct method. I'll have another try when I get the time and motivation.
This PR has sat here untouched for over 4 years - and that doesn't really help much as regards "motivation".

[sisyphus]

Some more digging, and inside uniq() in ListUtils.xs, there's the line of code (line 1376):

if(!SvOK(arg) || SvNOK(arg) || SvPOK(arg))

The segfault is happening when the !SvOK(arg) condition is being tested.

That's odd - because, in another script, I had already checked to see whether SvOK(arg) was true :

use warnings;
use strict;
use Inline C =><<'EOC';

int foo(SV * in) {
  if(!SvOK(in)) return 0;
  return 1;
}

EOC

my $obj = bless {}, __PACKAGE__;
print foo($obj);

That all ran fine, and produced the output of "1".
So there's apparently a problem with the way that this $obj is being accessed by uniq().

Hopefully there are people here who can pinpoint the problem.
I'll keep pursuing, but I think I might be getting into an area that's outside my comfort zone.

[zhmylove]

@sisyphus thanks for your research and help!

Actually, the issue is with (arg = AMG_CALLun(arg, int)) as SvAMAGIC(arg) returns TRUE.
I'll continue the investigation further and maybe create a fix for the problem.

So you think it'd be better to deliver it via another PR?
I can open the PR as soon as I find an apropriate resolution.

[zhmylove]

Done with it: #137

[sisyphus]

Looks good !!
#137 works well for me.