> * What is your evaluation of the proposal? +1 in general > * Is the problem being addressed significant enough to warrant a change to > Swift? Yes. > * Does this proposal fit well with the feel and direction of Swift? Yes, with some nitpicks detailed below. > * If you have used other languages or libraries with a similar feature, how > do you feel that this proposal compares to those? - > * How much effort did you put into your review? A glance, a quick reading, or > an in-depth study?
An afternoon of detailed study. As the author of a Swift package for arbitrary integers (https://github.com/lorentey/BigInt), I’m delighted by this proposal. I’m especially stoked about `FixedWidthInteger.doubleWidthMultiply`, which will likely lead to a measurable speedup. Why is there no `doubleWidthQuotientAndRemainder` or `doubleWidthDivide`, though? What is the intended behavior for `nthWord` for negative numbers? Can we prevent having to answer this question by e.g. moving this method down to UnsignedInteger? Big integer libs often use a signed magnitude representation; having to e.g. convert it to two’s complement on the fly to satisfy the API would be weird. (Also, the name `nthWord` seems unswifty to me.) Like others on this list, I also find signBitIndex confusing. The name does not make much sense for unsigned integers, or signed integers that do not use an embedded sign bit. Is it supposed to return the width (in bits) of the binary representation of the integer’s absolute value? (Why -1 for zero, then? What’s the signBitIndex for 1?) I can’t find any mention of the unary bitwise not (~) operation. It should be in FixedWidthInteger, right? Binary bitwise operations (or/and/xor) are in FixedWidthInteger, but I believe they can be implemented on big integers in a way that is perfectly consistent with fixed width integers. Admittedly, I don’t know of any generic algorithms that want these. To support big integers better, I think IntegerLiteralConvertible should also be updated at some point to work in terms of machine words. (Having to implement StringLiteralConvertible to provide source-level support for huge numbers works, but it isn’t great.) I can see myself typing foo.absoluteValue instead of abs(foo) all the time; if its use is to be discouraged, perhaps a less prominent name would be better. There is a typo in the Operators section: “Arithmetic” should read “Integer” in these two lines: public func % <T: Arithmetic>(lhs: T, rhs: T) -> T public func %= <T: Arithmetic>(lhs: inout T, rhs: T) Remark: The gyb is nice when used in moderation, but I find parts of the prototype overuse templating to a point that’s bordering on obfuscation. E.g., I found this code especially hard to read: https://github.com/apple/swift/blob/master/test/Prototypes/Integers.swift.gyb#L1016-L1033 <https://github.com/apple/swift/blob/master/test/Prototypes/Integers.swift.gyb#L1016-L1033> -- Karoly @lorentey > On 2016-06-23, at 02:52, Chris Lattner via swift-evolution > <[email protected]> wrote: > > Hello Swift community, > > The review of "SE-0104: Protocol-oriented integers" begins now and runs > through June 27. The proposal is available here: > > > https://github.com/apple/swift-evolution/blob/master/proposals/0104-improved-integers.md > > Reviews are an important part of the Swift evolution process. All reviews > should be sent to the swift-evolution mailing list at > > https://lists.swift.org/mailman/listinfo/swift-evolution > > or, if you would like to keep your feedback private, directly to the review > manager. > > What goes into a review? > > The goal of the review process is to improve the proposal under review > through constructive criticism and contribute to the direction of Swift. When > writing your review, here are some questions you might want to answer in your > review: > > * What is your evaluation of the proposal? > * Is the problem being addressed significant enough to warrant a change > to Swift? > * Does this proposal fit well with the feel and direction of Swift? > * If you have used other languages or libraries with a similar feature, > how do you feel that this proposal compares to those? > * How much effort did you put into your review? A glance, a quick > reading, or an in-depth study? > > More information about the Swift evolution process is available at > > https://github.com/apple/swift-evolution/blob/master/process.md > > Thank you, > > -Chris Lattner > Review Manager > > > _______________________________________________ > swift-evolution mailing list > [email protected] > https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
