Add signed(T), unsigned(T).

[NHDaly]

Converts Types into their corresponding signed/unsigned equivalents, maintaining storage size, but changing signed-ness.

Fixes #30444.

[NHDaly]

(Still needs tests, etc, but just wanted to see what y'all think first! 😄)

[JeffreySarnoff]

I use these extensions often. For the builtin types, I favor

unsigned(::Type{Int64}) = UInt64
unsigned(::Type{Float32}) = UInt32  # important for low level floating point work
unsigned(x::T) where {T<:Union{Int8..Int128}} = reinterpret(unsigned(T), x)
# etc and cover signed() and e.g. floating(::Type{UInt32}) = Float32 ..

[timholy]

I approve of adding this, once it has tests.

@JeffreySarnoff, I don't doubt being able to derive the "plain bits" type corresponding to a floating-point type is useful, but I don't think that operation can be called unsigned. Maybe call that planbitstype or something.

[NHDaly]

I approve of adding this, once it has tests.

:) Thanks Tim! I've added Tests. This will probably also need a News change before it could be merged.

@JeffreySarnoff, I don't doubt being able to derive the "plain bits" type corresponding to a floating-point type is useful, but I don't think that operation can be called unsigned. Maybe call that plainbitstype or something.

I have to agree with Tim about that. Converting a float to an int seems beyond the scope of unsigned/signed. Also, I think you could already achieve that with something like Int(3.0) / UInt(3.0)?

[NHDaly]

I changed all these tests to be === from ==, because they would be equal even if signed() did nothing. Lemme know if you don't like that! :)

Also, I was a bit confused by the @test_throws here. I clarified its behavior with this # Note, but maybe it's better to fix/clarify the test itself? I'm not reallly sure what it's testing though, unfortunately.

[timholy]

I don't understand that test either. Does git blame shed any light?

[NHDaly]

Looks like the test was added here: #11962, during v0.4.0.

Maybe the behavior was different back then, or maybe it was a mistake? (It was David's first PR ever after all! 😋) I'm happy to either remove the whole line or just leave the comment; whatever you prefer!

[timholy]

I don't understand that test either. Does git blame shed any light?

[NHDaly]

Also: is this news-worthy? I'm ambivalent on that, but here's a draft for what i can put there:

* There are new method overloads `unsigned(T::Type)` and `signed(T::Type)` that accept a `Type`,
  and return its corresponding signed/unsigned equivalent type ([#30445]).

[NHDaly]

(K, i added a NEWS entry based on your thumbsup)

[NHDaly]

I reran the tests, and they're passing now. :)

Please take another look when you get a chance! :)

[NHDaly]

Friendly ping, this should be good to merge. Please take another look?

[JeffreySarnoff]

I am wondering what benefit there is to avoiding symmetry in these two functional definitions.
To my eye, this is more natural and tends better toward future-proof:

unsigned(::Type{T}) where {T<:Unsigned} = T
unsigned(::Type{T}) where {T<:BitSigned} = typeof(unsigned(zero(T)))

signed(::Type{T}) where {T<:Signed} = T
signed(::Type{T}) where {T<:BitUnsigned} = typeof(signed(zero(T)))

(I don't have v1.2 available to run these .. if the foregoing does not work, there are bigger issues)

[NHDaly]

I am wondering what benefit there is to avoiding symmetry in these two functional definitions.

@JeffreySarnoff I completely agree. We discussed it above, here: #30445 (comment) -- the reason is to mirror the existing asymmetry in the definitions of unsigned and signed. As you pointed out there, I guess they're like that due to a difference regarding BigInt...

I opened #30473 to discuss this further. I think we should probably make signed and unsigned symmetric in the way you suggest. Either make them both take Signed and Unsigned, or both take BitSigned and BitUnsigned. I'd actually lean towards the first option i think. But yes, i agree, and i think we should change that for the existing signed/unsigned as well.

But that also seems orthogonal to this PR, which is why i separated it into #30473.

[JeffreySarnoff]

I want to see this resolved as much as you do (modulo keypress as unit of effort). There is nothing in my experience that suggests spackle over a conceptual inelegance is a winning approach -- there are situations wherein it is the only approach available, fortunately this is not so for us.

Rather than orthogonal concerns there is a dependance relationship. Get the underpinning correct and this PR is implemented with clarity, coherence and aplomb. Leaving the underpinning for resolution after the band-aid is applied ... well, who enjoys ripping off a bandage when the alternative is no bandage?

I don't mind that there are split into two PR. It is my considered view that we need to come to an agreement on the other first. I am willing to help do that.

[JeffreySarnoff]

I have a feeling that's not all. @timholy the first version of SaferIntegers was a conversion of your approach writing the pkg for Ints that round themselves -- and I put a premium on minimizing changes. I remember spending days on why you had to use discrepant approaches to the sogned and the unsigned types and wondering how to bring both under a shared umbrella Integer type that would support the expressiveness we like.

[NHDaly]

Now that #33459 was merged, I've (finally) updated this PR to the simple version suggested by @JeffreySarnoff above.
This is now fully symmetric, and simple. Thanks for the patience! :)

@JeffBezanson on the review of #33459, you noted that you're "really surprised we still have signed and unsigned. It seems like those should be x % Signed instead."
Is it okay that we move forward then with extending these methods in this way? I suppose instead of overloading like this, we could simply rename these functions to something else, like signedtype and unsignedtype (or signed_type and unsigned_type)?

[NHDaly]

Is it okay that we move forward then with extending these methods in this way? I suppose instead of overloading like this, we could simply rename these functions to something else, like signedtype and unsignedtype (or signed_type and unsigned_type)?

Though note that, the whole motivation for this PR was the observation that Base already includes an (undocumented) method for unsigned(::Type), just not one for signed(::Type):
#30444 (comment)

(Which, actually, this makes me wonder why i didn't see a method redefinition warning.. Ah, never mind yeah there is one. Lemme fix that now. EDIT: Fixed.)

So I guess if we rename these, either we'd want to keep that weird undocumented asymmetry, or remove the undocumented method (which is "breaking")?

[JeffreySarnoff]

fwiw I am happier with Unsigned(T) and Signed(T) than unsigned(T) and signed(T) for this role.

[NHDaly]

fwiw I am happier with Unsigned(T) and Signed(T) than unsigned(T) and signed(T) for this role.

I can see the appeal of that, but as a counter-argument, Signed and Unsigned read like constructors for types. It seems unexpected to me for the Unsigned constructor to return a value that is not <: Unsigned.

Perhaps the best name for these would simply be the more explicit:

• unsignedtype(T) and signedtype(T)?

[NHDaly]

I think that would be my vote now: unsignedtype(T) and signedtype(T). It doesn't address the question of why we even have those unsigned(x) and signed(x) functions laying around, but oh well such is life. No reason to pile on to that decision.

unsignedtype(T) and signedtype(T) is clear, explicit, and discoverable, i think.

[Keno]

This needs a rebase, but looks otherwise reasonable to me.

[NHDaly]

@Keno do you have a preference regarding renaming these to unsignedtype(T) and signedtype(T)? If i was to rewrite this PR, i would advocate for those names (except that there's already the [undocumented] unsigned(T), described in #30444)

[Keno]

If it's just about @JeffBezanson's comment regarding the value of version these functions, then I don't think it's a problem, because the type method doesn't quite have the same semantics. It's not really a value conversion, it's a type property query. So I'm fine with the PR as is. Did I miss another reason to use a different generic function?

[musm]

unsignedtype(T) and signedtype(T) is clear, explicit, and discoverable, i think.

I also agree that these names would be more descriptive. I'm not sure on the policy on renaming exported function in 1.x releases, but someone should chime in before we merge this.

[kimikage]

BTW, PR #34864 was merged. So, at least #30444 could be closed.

The discussion in this PR may be still worth continuing, but it seems to be a good idea to change the subject of this PR.

cc: @NHDaly, @JeffreySarnoff

[JeffreySarnoff]

what do you suggest?

[kimikage]

To be honest, I am satisfied with the current signed(T)/unsigned(T), so I don't feel the need to discuss new function names or interfaces.

However, I think that adding functions such as unsignedtype will affect PR #36526.

[JeffreySarnoff]

That makes sense. Certainly we should move the direction of these two so we are following one more consistent approach.

[StefanKarpinski]

Marked for triage to decide whether to do this or not.

[musm]

Sounds good, triage should also considered this PR and the methods names in light of #36526 , as brought up by @kimikage and @JeffreySarnoff

[JeffBezanson]

I believe we added this already.