<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Jordan,<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 23, 2016, at 1:50 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com" class="">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hey, standard library folks. Glad we're doing this one. :-)<div class=""><br class=""></div><div class="">- 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?</div></div></div></blockquote><div>Steve, I don’t remember exactly why we chose to do it this time. Can you answer?</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- 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?</div></div></div></blockquote>The same thing it does now — trap.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- Integer.init<T: Integer>(_:) currently says "if it is representable". It should say something like "trapping if it is not representable”.</div></div></div></blockquote>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?<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- 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 <i class="">defaults</i> to the min and max, but that's not implementable for a BigInt.</div></div></div></blockquote>It is always possible to pass bounds as an optional value and default to min…max in .none case.</div><div>So you are suggesting something like `init<T : Integer>(clamping: T, within bounds: Optional<ClosedRange<Self>> = nil)` then?</div><div>I don’t disagree. Looks useful, but I cannot imagine a good real world use for it, except for:</div><div><br class=""></div><div>extension Integer {</div><div> public func findClosest(to: Self, within bounds: ClosedRange<Self>) {</div><div> return Self(clamping: to, within: bounds)</div><div> }</div><div>}</div><div><br class=""></div><div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- nthWord should count "from least-significant to most-significant" rather than "from the right”.</div></div></div></blockquote>Will this definition work fine with different endiannesses? It needs some thinking, but I see your point.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- 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.</div></div></div></blockquote>I’ll add it. Thanks.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- 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.)</div></div></div></blockquote>There is a derived property that will return the width in words based on bitWidth and word size.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- It's also probably worth calling out <i class="">even more explicitly</i> that bitWidth is a <i class="">representation</i> property, not a <i class="">value</i> property. That is, a BigInt with the value "1" could have a bitWidth of 1, 8, or 128.</div></div></div></blockquote>Noted. Thanks.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- 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.</div></div></div></blockquote>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.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- For signed integers, does remainder(dividingBy:) have specified behavior for the sign of the result? See <a href="https://en.wikipedia.org/wiki/Modulo_operation" class="">https://en.wikipedia.org/wiki/Modulo_operation</a>.</div></div></div></blockquote>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.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class=""><div class="">- I do think having Swift.abs(_:) and Integer.absoluteValue is confusing, but I don't know what to do about it.</div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class=""><br class=""></div><div class="">- 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).</div></div></div></blockquote>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.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">- Is there a requirement about left-shifting into the sign bit, for '<<' and for '&<<‘?</div></div></div></blockquote>The current behavior is to not do anything special about the sign bit. So that (64 as Int8) << 1 would result in a -128.</div><div><br class=""></div><div>I should probably add a general note somewhere in the proposal that “unless specifically mentioned, the behavior will remain consistent with the existing implementation”.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div><div class="">- What is the ArithmeticOverflow type?</div></div></div></div></blockquote>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.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div><div class="">- When does the remainder operation overflow? (I just can't remember.)</div></div></div></div></blockquote>Discussed in a separate email. The only case where it should trap is ‘division by zero’, so this part is subjected to changes.</div><div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div><div class="">- I feel a little weird having "someValue.and(mask)". Maybe bitwiseAnd or bitwiseAND to be more explicit?</div></div></div></div></blockquote>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.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div></div><div class="">- 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?</div></div></div></blockquote>There is a section about shifts: <a href="https://github.com/apple/swift-evolution/blob/master/proposals/0104-improved-integers.md#a-note-on-bit-shifts" class="">https://github.com/apple/swift-evolution/blob/master/proposals/0104-improved-integers.md#a-note-on-bit-shifts</a></div><div>Let me know if you think it is insufficient.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">I think that's about it. Great work, all!</div></div></div></blockquote></div><div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Jordan</div></div></div></blockquote><br class=""></div><div><div>Thank you for a thoughtful review!</div><div>Max</div></div><br class=""></div></body></html>