Merge pull request #11044 from apple/rationalize-bitshifting

[stdlib] Rationalize bitshifting in protocols

Author: moiseev

stdlib/public/core/DoubleWidth.swift.gyb

@@ -89,7 +89,7 @@ public struct DoubleWidth<Base : FixedWidthInteger> :
       // At this point we know source's bitWidth > Base.bitWidth, or else
       // we would've taken the first branch.
       let lowInT = source & T(~0 as Low)
-      let highInT = source &>> numericCast(High.bitWidth)
+      let highInT = source >> High.bitWidth
 
       let low = Low(lowInT.magnitude)
       guard let high = High(exactly: highInT) else { return nil }

stdlib/public/core/Integers.swift.gyb

@@ -1489,14 +1489,25 @@ ${assignmentOperatorComment(x.operator, False)}
   /// - Complexity: O(1).
   static prefix func ~ (_ x: Self) -> Self
 
-% for x in binaryBitwise + maskingShifts:
+% for x in binaryBitwise:
 ${operatorComment(x.operator, False)}
   static func ${x.operator}(_ lhs: Self, _ rhs: Self) -> Self
 
 ${assignmentOperatorComment(x.operator, False)}
   static func ${x.operator}=(_ lhs: inout Self, _ rhs: Self)
 % end
 
+% for x in maskingShifts:
+${operatorComment(x.nonMaskingOperator, False)}
+  static func ${x.nonMaskingOperator}<RHS: BinaryInteger>(
+    _ lhs: Self, _ rhs: RHS
+  ) -> Self
+
+${assignmentOperatorComment(x.nonMaskingOperator, False)}
+  static func ${x.nonMaskingOperator}=<RHS: BinaryInteger>(
+    _ lhs: inout Self, _ rhs: RHS)
+% end
+ 
   /// Returns the quotient and remainder of this value divided by the given
   /// value.
   ///
@@ -1549,8 +1560,9 @@ extension BinaryInteger {
     return (self / rhs, self % rhs)
   }
 
-  % for x in binaryBitwise + maskingShifts:
+  % for x in binaryBitwise:
 
+  // Homogeneous
 ${operatorComment(x.operator, False)}
   @_transparent
   public static func ${x.operator} (lhs: Self, rhs: Self) -> Self {
@@ -1561,24 +1573,18 @@ ${operatorComment(x.operator, False)}
 
   % end
 
-  % for x in maskingShifts:
-
-${operatorComment(x.operator, False)}
-  public static func ${x.operator} <
-    Other : BinaryInteger
-  >(lhs: Self, rhs: Other) -> Self {
-    return lhs ${x.operator} Self(extendingOrTruncating: rhs)
-  }
-
-${assignmentOperatorComment(x.operator, False)}
-  @_transparent
-  public static func ${x.operator}= <
-    Other : BinaryInteger
-  >(lhs: inout Self, rhs: Other) {
-    lhs = lhs ${x.operator} rhs
+% for x in maskingShifts:
+  // Heterogeneous non-masking shift in terms of shift-assignment
+${operatorComment(x.nonMaskingOperator, False)}
+  public static func ${x.nonMaskingOperator}<RHS: BinaryInteger>(
+    _ lhs: Self, _ rhs: RHS
+  ) -> Self {
+    var r = lhs
+    r ${x.nonMaskingOperator}= rhs
+    return r
   }
+% end
 
-  % end
 }
 
 // Strideable conformance
@@ -1835,50 +1841,6 @@ extension BinaryInteger {
   }
 }
 
-//===----------------------------------------------------------------------===//
-//===--- BinaryInteger smart shifts ---------------------------------------===//
-//===----------------------------------------------------------------------===//
-// FIXME(integers): uncomment once <rdar://problem/29643515> gets fixed
-#if false
-extension BinaryInteger {
-%   for x in maskingShifts:
-  @_transparent
-  public static func ${x.nonMaskingOperator} <
-    Other : BinaryInteger
-  >(lhs: Self, rhs: Other) -> Self {
-    var lhs = lhs
-    lhs ${x.nonMaskingOperator}= rhs
-    return lhs
-  }
-
-  // It is hard to imagine overshift to the left in an arbitrarily sized
-  // integer, but shifting too far to the right and negative shift cases are
-  // supported.
-%     reversedOperator = x.operator.translate(maketrans('<>', '><'))
-%     isRightShift = '>' in x.operator
-  @_transparent
-  public static func ${x.nonMaskingOperator}= <
-    Other : BinaryInteger
-  >(lhs: inout Self, rhs: Other) {
-    if rhs < (0 as Other) {
-      lhs ${reversedOperator}= (0 - rhs)
-      return
-    }
-%     if isRightShift:
-    let overshift = Self.isSigned
-      ? (lhs < (0 as Self) ? ~(0 as Self) : 0 )
-      : 0
-    if rhs >= lhs.bitWidth {
-      lhs = overshift
-      return
-    }
-%     end
-    lhs ${x.operator}= Self(extendingOrTruncating: rhs)
-  }
-%   end
-}
-#endif
-
 //===----------------------------------------------------------------------===//
 //===--- FixedWidthInteger ------------------------------------------------===//
 //===----------------------------------------------------------------------===//
@@ -2105,6 +2067,15 @@ ${overflowOperationComment(x.operator)}
 
   /// A representation of this integer with the byte order swapped.
   var byteSwapped: Self { get }
+
+% for x in maskingShifts:
+${operatorComment(x.operator, False)}
+  static func ${x.operator}(_ lhs: Self, _ rhs: Self) -> Self
+
+${assignmentOperatorComment(x.operator, False)}
+  static func ${x.operator}=(_ lhs: inout Self, _ rhs: Self)
+% end
+
 }
 
 extension FixedWidthInteger {
@@ -2162,6 +2133,37 @@ extension FixedWidthInteger {
     return byteSwapped
 #endif
   }
+  
+  % for x in maskingShifts:
+  
+  // Homogeneous masking shift
+${operatorComment(x.operator, False)}
+  @_transparent
+  public static func ${x.operator} (lhs: Self, rhs: Self) -> Self {
+    var lhs = lhs
+    lhs ${x.operator}= rhs
+    return lhs
+  }
+
+
+  // Heterogeneous masking shift
+${operatorComment(x.operator, False)}
+  public static func ${x.operator} <
+    Other : BinaryInteger
+  >(lhs: Self, rhs: Other) -> Self {
+    return lhs ${x.operator} Self(extendingOrTruncating: rhs)
+  }
+
+  // Heterogeneous masking shift assignment
+${assignmentOperatorComment(x.operator, False)}
+  @_transparent
+  public static func ${x.operator}= <
+    Other : BinaryInteger
+  >(lhs: inout Self, rhs: Other) {
+    lhs = lhs ${x.operator} rhs
+  }
+
+  % end
 }
 
 //===----------------------------------------------------------------------===//
@@ -2190,20 +2192,6 @@ ${operatorComment(x.nonMaskingOperator, True)}
     return lhs
   }
 
-// FIXME(integers): uncommenting this overload results in a compiler not being
-// able to typecheck expression like `(int64 >> 8) & 0xFF`.
-#if false
-  @_transparent
-  public static func ${x.nonMaskingOperator} (_ lhs: Self, _ rhs: Int) -> Self {
-    return ${x.helper}(lhs, rhs)
-  }
-
-  @_transparent
-  public static func ${x.nonMaskingOperator}= (_ lhs: inout Self, _ rhs: Int) {
-    lhs = ${x.helper}(lhs, rhs)
-  }
-#endif
-
   @_transparent
   public static func ${x.nonMaskingOperator}= <
     Other : BinaryInteger

test/Prototypes/BigInt.swift

@@ -1090,24 +1090,26 @@ public struct _BigInt<Word: FixedWidthInteger & UnsignedInteger> :
     data.removeFirst(Swift.min(data.count, words))
   }
 
-  public static func <<=(lhs: inout _BigInt, rhs: Int) {
+  public static func <<= <RHS : BinaryInteger>(lhs: inout _BigInt, rhs: RHS) {
     defer { lhs._checkInvariants() }
     guard rhs != 0 else { return }
     guard rhs > 0 else {
-      lhs >>= -rhs
+      lhs >>= 0 - rhs
       return
     }
 
+    let wordWidth = RHS(Word.bitWidth)
+    
     // We can add `rhs / bits` extra words full of zero at the low end.
-    let extraWords = rhs / Word.bitWidth
+    let extraWords = Int(rhs / wordWidth)
     lhs._data.reserveCapacity(lhs._data.count + extraWords + 1)
     _BigInt._shiftLeft(&lhs._data, byWords: extraWords)
 
     // Each existing word will need to be shifted left by `rhs % bits`.
     // For each pair of words, we'll use the high `offset` bits of the
     // lower word and the low `Word.bitWidth - offset` bits of the higher
     // word.
-    let highOffset = rhs % Word.bitWidth
+    let highOffset = Int(rhs % wordWidth)
     let lowOffset = Word.bitWidth - highOffset
 
     // If there's no offset, we're finished, as `rhs` was a multiple of
@@ -1126,19 +1128,20 @@ public struct _BigInt<Word: FixedWidthInteger & UnsignedInteger> :
     lhs._standardize()
   }
 
-  public static func >>=(lhs: inout _BigInt, rhs: Int) {
+  public static func >>= <RHS : BinaryInteger>(lhs: inout _BigInt, rhs: RHS) {
     defer { lhs._checkInvariants() }
     guard rhs != 0 else { return }
     guard rhs > 0 else {
-      lhs <<= -rhs
+      lhs <<= 0 - rhs
       return
     }
 
     var tempData = lhs._dataAsTwosComplement()
 
+    let wordWidth = RHS(Word.bitWidth)
     // We can remove `rhs / bits` full words at the low end.
     // If that removes the entirety of `_data`, we're done.
-    let wordsToRemove = rhs / Word.bitWidth
+    let wordsToRemove = Int(rhs / wordWidth)
     _BigInt._shiftRight(&tempData, byWords: wordsToRemove)
     guard tempData.count != 0 else {
       lhs = lhs.isNegative ? -1 : 0
@@ -1149,7 +1152,7 @@ public struct _BigInt<Word: FixedWidthInteger & UnsignedInteger> :
     // For each pair of words, we'll use the low `offset` bits of the
     // higher word and the high `_BigInt.Word.bitWidth - offset` bits of
     // the lower word.
-    let lowOffset = rhs % Word.bitWidth
+    let lowOffset = Int(rhs % wordWidth)
     let highOffset = Word.bitWidth - lowOffset
 
     // If there's no offset, we're finished, as `rhs` was a multiple of
@@ -1169,18 +1172,6 @@ public struct _BigInt<Word: FixedWidthInteger & UnsignedInteger> :
     tempData[tempData.count - 1] >>= lowOffset
     lhs = _BigInt(_twosComplementData: tempData)
   }
-
-  public static func <<(lhs: _BigInt, rhs: Int) -> _BigInt {
-    var lhs = lhs
-    lhs <<= rhs
-    return lhs
-  }
-
-  public static func >>(lhs: _BigInt, rhs: Int) -> _BigInt {
-    var lhs = lhs
-    lhs >>= rhs
-    return lhs
-  }
 }
 
 //===--- Bit --------------------------------------------------------------===//
@@ -1788,7 +1779,8 @@ BigInt8Tests.test("Bitshift") {
 
   (x, y) = (BigInt(UInt.max), UInt.max)
   for i in 0...64 {   // test 64-bit shift, should both be zero
-    expectTrue(x >> i == y >> i)
+    expectTrue(x >> i == y >> i,
+    "\(x) as \(type(of:x)) >> \(i) => \(x >> i)  !=  \(y) as \(type(of:y)) >> \(i) => \(y >> i)")
   }
 
   x = BigInt(-1)