[swift-evolution] [swift-evolution-announce] [Review] SE-0104: Protocol-oriented integers

[Max Moiseev]

Hi Jordan,

> On Jun 23, 2016, at 1:50 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
> Hey, standard library folks. Glad we're doing this one. :-)
> 
> - I remain unconvinced that defining an Arithmetic that includes both exact and floating-point numbers is a good idea. All of the arguments from Swift 1 and 2 about why we didn't include this still seem relevant. To phrase it in generic programming terms, what algorithm would be generic over Arithmetic?
Steve, I don’t remember exactly why we chose to do it this time. Can you answer?

> 
> - What is Integer.init<T: FloatingPoint>(_:) supposed to do if the floating-point value is larger than the maximum representable integer? Smaller than the minimum? (As a special case, negative, when the integer type is unsigned?) Infinity? NaN?
The same thing it does now — trap.

> 
> - Integer.init<T: Integer>(_:) currently says "if it is representable". It should say something like "trapping if it is not representable”.
There is a comment saying that this is the precondition that must be checked by the caller. The initializer will trap but this is the implementation detail, isn’t it?
> 
> - I find it odd that Integer.init(clamping:) privileges the bounds of fixed-width integers. I was going to suggest it should take a range to clamp to that defaults to the min and max, but that's not implementable for a BigInt.
It is always possible to pass bounds as an optional value and default to min…max in .none case.
So you are suggesting something like `init<T : Integer>(clamping: T, within bounds: Optional<ClosedRange<Self>> = nil)` then?
I don’t disagree. Looks useful, but I cannot imagine a good real world use for it, except for:

extension Integer {
  public func findClosest(to: Self, within bounds: ClosedRange<Self>) {
    return Self(clamping: to, within: bounds)
  }
}

> 
> - nthWord should count "from least-significant to most-significant" rather than "from the right”.
Will this definition work fine with different endiannesses? It needs some thinking, but I see your point.
> 
> - As mentioned before, it sounds like Integer requires a two's complement representation (if only so the result of nthWord can be interpreted correctly). That should probably be in the doc comment for the protocol.
I’ll add it. Thanks.
> 
> - Why is bitWidth in bits but nthWord in words? (I know there's a good answer to this, but using them together seems like it will be common.)
There is a derived property that will return the width in words based on bitWidth and word size.
> 
> - It's also probably worth calling out even more explicitly that bitWidth is a representation property, not a value property. That is, a BigInt with the value "1" could have a bitWidth of 1, 8, or 128.
Noted. Thanks.

> 
> - What does signBitIndex return if self is positive? I ask because it's just not in the doc comment, but thinking about the answer made it obvious that the correct return value for 0 is 0.
The doc comment right now describes the actual behavior from the prototype implementation. I plan to document this particular property better after cleaning up the prototype.

> 
> - For signed integers, does remainder(dividingBy:) have specified behavior for the sign of the result? See https://en.wikipedia.org/wiki/Modulo_operation <https://en.wikipedia.org/wiki/Modulo_operation>.
The behavior should remain the same as it is now (the sign is preserved). I guess this just should be explicitly mentioned somewhere. Will do.
> 
> - I do think having Swift.abs(_:) and Integer.absoluteValue is confusing, but I don't know what to do about it.
> 
> 
> - Why are bitwise operations limited to fixed-width integers? I see "The only difference is that because shifting left truncates the high bits of fixed-width integers, it is hard to define what a left shift would mean to an arbitrary-precision integer" further down, but I would just assume it wouldn't truncate (i.e. it would be a pure multiplication by two).
Exactly. It won’t truncate and we considered it to be a sufficient difference in behavior to not allow it to be used in the context over all integers.
> 
> - Is there a requirement about left-shifting into the sign bit, for '<<' and for '&<<‘?
The current behavior is to not do anything special about the sign bit. So that (64 as Int8) << 1 would result in a -128.

I should probably add a general note somewhere in the proposal that “unless specifically mentioned, the behavior will remain consistent with the existing implementation”.

> 
> - What is the ArithmeticOverflow type?
It is an enum with 2 cases, just like Optional<()>, but with more specific case name. If not for the explicitness, a simple Bool value could have been used.
> 
> - When does the remainder operation overflow? (I just can't remember.)
Discussed in a separate email. The only case where it should trap is ‘division by zero’, so this part is subjected to changes.
> 
> - I feel a little weird having "someValue.and(mask)". Maybe bitwiseAnd or bitwiseAND to be more explicit?
Doesn’t return type hint at the ‘bitwise' and not logical nature of these operations? Besides, I would expect operators to be used instead of actual protocol functions.
> 
> - maskingShiftLeft/Right seem underspecified in their doc comments. Why can't the protocol requirement just assume the shift amount has already been masked, instead of performing the masking themselves? Is it because we won't be able to optimize that away?
There is a section about shifts: https://github.com/apple/swift-evolution/blob/master/proposals/0104-improved-integers.md#a-note-on-bit-shifts
Let me know if you think it is insufficient.
> 
> I think that's about it. Great work, all!

> Jordan

Thank you for a thoughtful review!
Max

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160623/4789fe9e/attachment.html>