[perl #33603] Undef != Undef
Simon Glover
# Please include the string: [perl #33603]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org:80/rt3/Ticket/Display.html?id=33603 >
This code:
new P0, .Undef
new P1, .Undef
eq P0, P1, L1
print "not "
L1: print "ok\n"
end
prints "not ok". Should it? If Parrot considers every Undef PMC to
be distinct, it's going to make tasks like comparing arrays with
large numbers of undefined elements much fiddlier than it should be.
For instance, the fact that Undef != Undef means that this code:
new P0, .ResizablePMCArray
set P0, 1
clone P1, P0
eq P0, P1, L1
print "not "
L1: print "ok"
print "\n"
end
prints "not ok".
Simon
Leopold Toetsch
> This code:
> new P0, .Undef
> new P1, .Undef
> eq P0, P1, L1
> print "not "
> L1: print "ok\n"
> end
> prints "not ok". Should it?
That depends ;)
> ... If Parrot considers every Undef PMC to
> be distinct, it's going to make tasks like comparing arrays with
> large numbers of undefined elements much fiddlier than it should be.
Yep. Currently there is no chance to change that. The semantics of unary
and binary operators need an existing destination PMC. Usual code for
infix '+' is:
$P0 = new Undef
$P0 = a + b
So *currently* the Undefs have to be distinct.
But as outlined several times, the current behavior isn't that one that
HLLs have. Overloaded operators are functions (multi subs) that return
new values. For consistency we really should do the same with operators.
Additionally we currently can't have singletons as a result of an
operator. This isn't only a serious restriction but can additionally be
much more expensive then the Undef issue.
We should have:
$P0 = cmp a, b
with the return result being a singleton True of False PMC. Now a HLL
compiler is forced to emit:
$I0 = cmp a, b
$P0 = new Boolean
$P0 = $I0
which is three times the code size and it needs a new PMC for each compare.
> For instance, the fact that Undef != Undef means that this code:
> new P0, .ResizablePMCArray
> set P0, 1
> clone P1, P0
> eq P0, P1, L1
> print "not "
> L1: print "ok"
> print "\n"
> end
> prints "not ok".
This is a different issue. The ResizablePMCArray doesn't properly
inherit the is_equal multi method from FixedPMCArray and as far as I can
see, there is no Undef involved at all. Empty array slots are filled
with PMCNULL, which is a singleton. The example works for FixedPMCArray.
> Simon
leo
Sam Ruby
> Simon Glover <parrotbug...@parrotcode.org> wrote:
>
>> This code:
>
>> new P0, .Undef
>> new P1, .Undef
>> eq P0, P1, L1
>> print "not "
>>L1: print "ok\n"
>> end
>
>> prints "not ok". Should it?
>
> That depends ;)
>
>> ... If Parrot considers every Undef PMC to
>> be distinct, it's going to make tasks like comparing arrays with
>> large numbers of undefined elements much fiddlier than it should be.
>
> Yep. Currently there is no chance to change that.
If the desire is that two new .Undef values are to be considered equal,
then the attached patch achieves that.
- Sam Ruby
Simon Glover
On Fri, 31 Dec 2004, Leopold Toetsch wrote:
> Simon Glover <parrotbug...@parrotcode.org> wrote:
>
> > new P0, .ResizablePMCArray
> > set P0, 1
> > clone P1, P0
> > eq P0, P1, L1
> > print "not "
> > L1: print "ok"
> > print "\n"
> > end
>
> > prints "not ok".
>
> This is a different issue. The ResizablePMCArray doesn't properly
> inherit the is_equal multi method from FixedPMCArray and as far as I can
> see, there is no Undef involved at all. Empty array slots are filled
> with PMCNULL, which is a singleton. The example works for FixedPMCArray.
In the case of the ResizablePMCArray, the Undef comes into it because
of this loop in is_equal():
for (j = 0; j < n; ++j) {
PMC *item1, *item2;
item1 = DYNSELF.get_pmc_keyed_int(j);
item2 = VTABLE_get_pmc_keyed_int(INTERP, value, j);
if (item1 == item2)
continue;
if (!mmd_dispatch_i_pp(INTERP, item1, item2, MMD_EQ))
return 0;
}
and this code in get_pmc_keyed_int:
data = PMC_data(SELF);
if (data[key] == PMCNULL)
data[key] = pmc_new(INTERP, enum_class_Undef);
return data[key];
When is_equal calls get_pmc_keyed_int to do the comparison, it creates
and returns an Undef PMC, rather than simply returing PMCNULL, and
since Undef != Undef, this ultimately causes the comparison to fail.
In the case of FixedPMCArray, the test passes because the
implementation of get_pmc_keyed_int doesn't have the test for PMCNULL
-- instead, it just does:
data = (PMC **)PMC_data(SELF);
return data[key];
Hence, if the entry is PMCNULL, the code actually returns PMCNULL,
and since PMCNULL == PMCNULL, the comparison succeeds and the test
passes.
I guess the real question is should we be creating that Undef PMC;
i.e. should fetching an undefined element return a fully-fledged
Undef value, or simply a PMCNULL, which may later be promoted to a
real Undef?
Simon
Leopold Toetsch
> On Fri, 31 Dec 2004, Leopold Toetsch wrote:
>> This is a different issue. The ResizablePMCArray doesn't properly
>> inherit the is_equal multi method from FixedPMCArray and as far as I can
>> see, there is no Undef involved at all. Empty array slots are filled
>> with PMCNULL, which is a singleton. The example works for FixedPMCArray.
> and this code in get_pmc_keyed_int:
> data = PMC_data(SELF);
> if (data[key] == PMCNULL)
> data[key] = pmc_new(INTERP, enum_class_Undef);
> return data[key];
> When is_equal calls get_pmc_keyed_int to do the comparison, it creates
> and returns an Undef PMC, rather than simply returing PMCNULL, and
> since Undef != Undef, this ultimately causes the comparison to fail.
Ouch. That looks very broken. It not only doesn't just compare the
values (I see no reason, why the NULL PMC shouldn't have a is_equal and
some other methods) - it also changes the array contents.
> I guess the real question is should we be creating that Undef PMC;
> i.e. should fetching an undefined element return a fully-fledged
> Undef value, or simply a PMCNULL, which may later be promoted to a
> real Undef?
The semantics of core aggregates have to be defined with respect to
such operations in a consistent way. We can do it either way ...
> Simon
leo
Joshua Hoblitt via RT
>
>
> This code:
>
> new P0, .Undef
> new P1, .Undef
> print "not "
> end
>
> prints "not ok". Should it? If Parrot considers every Undef PMC to
> be distinct, it's going to make tasks like comparing arrays with
> large numbers of undefined elements much fiddlier than it should be.
>
>
> set P0, 1
> clone P1, P0
> eq P0, P1, L1
> print "not "
> L1: print "ok"
> print "\n"
> end
>
> prints "not ok".
>
>
>
>
Can we get a design descision as to wheather or not Undef == Undef will
ever have any meaning so we can close out this bug? The current beahvior of
./parrot foo.pasm
MMD function __is_equal not found for types (78, 78)
current instr.: '(null)' pc 6 (foo.pasm:3)
seems wrong to me any way you look at it.
-J
--
Joshua Isom
have on sorting? Although for sorting, I guess that NaN != NaN would
have the some issue, but undef values in an array are more likely.