Skip to content
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.

Sign up for GitHub

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

Jump to bottom

make BigInt <: Signed #23473

Merged
merged 1 commit into from
Sep 7, 2017
Merged

make BigInt <: Signed #23473

merged 1 commit into from
Sep 7, 2017

Conversation

rfourquet
Copy link
Member

Even if Signed has no docstring, it seems natural for me to have BigInt <: Signed (I am not alone: #19182 (comment)). It also seems good to track places which assume that a subtype of Signed has a sizeof or a typemax.

@rfourquet rfourquet added the domain:bignums BigInt and BigFloat label Aug 27, 2017
@StefanKarpinski
Copy link
Sponsor Member

I would have thought there were some assumptions of a two's complement representation here, but maybe there isn't. It makes me wonder if the hierarchy shouldn't be just Unsigned <: Integer and replace Signed with Integer everywhere.

@JeffBezanson
Copy link
Sponsor Member

It's hard to think of a good example, but my gut reaction is that Signed could be useful for requiring that a type support negative numbers. If Signed is merged with Integer then all you can express with types is "might support negative numbers" and "definitely doesn't support negative numbers", which feels incomplete.

@rfourquet
Copy link
Member Author

I would have thought there were some assumptions of a two's complement representation here

FWIW, GMP has (mostly?) the semantics of two's complement representation, even if the implementation is different.

As for merging Signed with Integer, I'm not clear yet; the name could be Signed instead of Integer, so that it would mean "does support negative numbers". But we would then probably have a const Integer = Union{Signed,Unsigned} alias, as this would be needed all over the place.

@rfourquet
Copy link
Member Author

Is someone opposed to this change?

@StefanKarpinski
Copy link
Sponsor Member

No, this seems sane and not very disruptive. The restrictions of methods to BitSigned seems to generally be more correct anyway.

@rfourquet rfourquet force-pushed the rf/Signed-BigInt branch 2 times, most recently from 9766934 to 5fedb70 Compare September 5, 2017 12:13
@rfourquet rfourquet merged commit b83c228 into master Sep 7, 2017
@rfourquet rfourquet deleted the rf/Signed-BigInt branch September 7, 2017 14:27
@omus omus mentioned this pull request Oct 23, 2017
Closed
omus added a commit to JuliaMath/FixedPointDecimals.jl that referenced this pull request Oct 24, 2017
@omus
Use only bits types for CONTAINER_TYPES
3466e86 
A change introduced in Julia 0.7 made BigInt a subtype of Signed. The
change resulted in the code including BigInt in the CONTAINER_TYPES
which resulted in an exceptions since BigInt is not a bits type.

JuliaLang/julia#23473
This was referenced Oct 24, 2017
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StefanKarpinski StefanKarpinski StefanKarpinski approved these changes

Assignees
No one assigned
Labels
domain:bignums BigInt and BigFloat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rfourquet @StefanKarpinski @JeffBezanson