Incorrect behavior of typep on complex numbers in compiled code

Bug #1733400 reported by Paul F. Dietz
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
Undecided
Unassigned

Bug Description

(funcall (compile nil '(lambda (x) (typep (if x #c(0 -1/2) nil) '(complex (integer -1 0))))) t)
==>
NIL

(funcall (compile nil '(lambda (x) (declare (notinline typep)) (typep (if x #c(0 -1/2) nil) '(complex (integer -1 0))))) t)
==>
T

summary: - Strange behavior of typep on complex numbers in compiled code
+ Incorrect behavior of typep on complex numbers in compiled code
Revision history for this message
Stas Boukarev (stassats) wrote :

8926ae335050ee4572140c3467410b71cfbb8b3a

Changed in sbcl:
status: New → Fix Committed
Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

(upgraded-complex-part-type 'integer) is RATIONAL, so I think you fixed the wrong thing. The notinline test was correct.

Sorry I didn't make this clear.

Revision history for this message
Stas Boukarev (stassats) wrote :

I fixed it to the intention of SBCL's typep, which may not intersect with some interpretations of the standard.

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote : Re: [Bug 1733400] Re: Incorrect behavior of typep on complex numbers in compiled code

Stas Boukarev <email address hidden> writes:

> I fixed it to the intention of SBCL's typep, which may not intersect
> with some interpretations of the standard.

I'm not convinced, but in case you are, can you describe sbcl's
intention of typep in this instance well enough for it to go in the
manual?

Christophe

Revision history for this message
Stas Boukarev (stassats) wrote :

It was only checking the realpart to be an integer, the fix is checking both.
Just as the standard is saying that for (typep object '(complex type-specifier)) (and (typep realpart 'type-specifier) (typep imagpart 'type-specifier)) must hold true. Don't think anything should be document beyond that.

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote :

Stas Boukarev <email address hidden> writes:

> It was only checking the realpart to be an integer, the fix is checking both.
> Just as the standard is saying that for (typep object '(complex
> type-specifier)) (and (typep realpart 'type-specifier) (typep imagpart
> 'type-specifier)) must hold true. Don't think anything should be
> document beyond that.

But I don't think that's implied! I think the point of upgrading is
that

  (typep y '(complex X))

means

  (and (typep (realpart y) `(upgraded-complex-part-type ',X))
       (typep (imagpart y) `(upgraded-complex-part-type ',X)))

Of course this is all a mess because there's no top to this type
lattice, and there's a hidden bottom when you consider X = (EQL 0)

CLHS says that we can think of (complex X) for X being a subtype of
REAL. It also says that actual complex number objects have real and
imaginary parts that are either both rational or both of the same
floating point type.

REAL is a subtype of REAL, or if you think the Elder Standardizers
really meant "proper subtype" (REAL -1 1) is a proper subtype of REAL.
However, there can be no single upgraded complex part type for
(upgraded-complex-part-type '(real -1 1)) because the complex could be
of a float flavour or a rational flavour depending on what you feed
the COMPLEX function. Probably the best answer in this case is to
return REAL from UPGRADED-COMPLEX-PART-TYPE, and to treat (COMPLEX REAL)
as equivalent to a union of our concrete complex types, much like
(VECTOR *) is effectively a union of all the specialized vector types.

But wait, there's more. (EQL 0) is a subtype of REAL, but there are *no*
complex numbers that you can get from feeding objects of type (EQL 0) to
the COMPLEX function. This implies that the upgraded-complex-part-type
of (EQL 0) should be NIL, but of course this breaks the requirement that
the result of upgraded-complex-part-type be a supertype of the given
type specifier. We currently return (EQL 0) as a cunning hack and
recognize (COMPLEX (EQL 0)) to be NIL as a type specifier.

I think the right answer to fix Paul's original test case is that since
we do not claim that we have a (COMPLEX INTEGER) specialized
representation, we shouldn't test for that in %%TYPEP. The code in
%%TYPEP is/was trying to be clever about handling reals and complexes in
the same code path; I think that detangling that will probably make
things easier to understand. And, yes, this means that #c(1/2 1/2) is of
type '(complex integer), somewhat analgously to how
(make-array 1 :element-type '(integer 1 255) :initial-element 0) is of
type '(simple-array (*) (integer 0 255))

Christophe

Revision history for this message
Stas Boukarev (stassats) wrote :

I don't think that there's actually any ambiguity anymore.
"(typep object '(complex type-specifier)) returns true for all complex numbers that can result from giving numbers of type type-specifier to the function
complex, plus all other complex numbers of the same specialized representation. Both the real and the imaginary parts of any such complex number must
satisfy:

 (typep realpart 'type-specifier)
 (typep imagpart 'type-specifier)"

None of the sentences contradict each other, but when you combine them all you get a restricted space of (and (typep realpart 'type-specifier) (typep imagpart 'type-specifier)) it was probably worded that way to handle (complex float), and it's even stated that array and complex types are handled differently.
"The small differences between the subtypep specification for the array and complex types are necessary because there is no creation function for complexes
which allows the specification of the resultant part type independently of the actual types of the parts."

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote :

Stas Boukarev <email address hidden> writes:

> I don't think that there's actually any ambiguity anymore.
> "(typep object '(complex type-specifier)) returns true for all complex
> numbers that can result from giving numbers of type type-specifier to
> the function
> complex, plus all other complex numbers of the same specialized
> representation. Both the real and the imaginary parts of any such
> complex number must
> satisfy:
>
> (typep realpart 'type-specifier)
> (typep imagpart 'type-specifier)"

Oh wow, I had completely not seen this on the page for TYPEP. I'd been
assuming that everything I needed was on the System Class COMPLEX page
:-(

> None of the sentences contradict each other, but when you combine them
> all you get a restricted space of (and (typep realpart
> 'type-specifier) (typep imagpart 'type-specifier))

I now think that the two sentences you've quoted above do contradict
each other, except if (upgraded-complex-part-type X) = X for all X.
Otherwise the upgrading brings in complexes (by the first sentence)
whose components don't match the type specifier (contradicting the
second sentence)

Maybe a trivial upgraded-complex-part-type is what we need to do. It
wouldn't be the worst outcome, I suppose, but it's not great. I'm
pretty sure the standardizers weren't thinking about exotic values of X
but the text is what it is.

> it was probably worded that way to handle (complex float), and it's
> even stated that array and complex types are handled differently.
> "The small differences between the subtypep specification for the
> array and complex types are necessary because there is no creation
> function for complexes which allows the specification of the resultant
> part type independently of the actual types of the parts."

This is talking about subtypep, not typep. I know they're related... I
agree that the difference looks like it's intending that (complex real)
is OK and means the union of all complexes (and (complex float) is the
union of all floaty complexes).

Christophe

Revision history for this message
Stas Boukarev (stassats) wrote :

upgraded-complex-part-type is an identity on clisp, although it defeats its purpose "to reveal how an implementation does its upgrading." But if any change is really required the change to upgraded-complex-part-type is the on i can live with, since nobody really uses it or knows about it.

Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

clisp also deviates from the standard in that (complex 1 1.0) doesn't convert the integer real part to float. As I recall they do document this.

Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.