stdlib: disable an incorrect String comparison optimization

stdlib/private/StdlibUnicodeUnittest/CMakeLists.txt

@@ -9,6 +9,7 @@ add_swift_library(swiftStdlibUnicodeUnittest ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES}
   # This file should be listed the first.  Module name is inferred from the
   # filename.
   StdlibUnicodeUnittest.swift
+  Collation.swift
 
   SWIFT_MODULE_DEPENDS StdlibUnittest
   SWIFT_COMPILE_FLAGS ${swift_stdlib_unittest_compile_flags}

stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb

@@ -1960,7 +1960,8 @@ public func checkEquatable<Instances : Collection>(
   _checkEquatableImpl(
     Array(instances),
     oracle: { oracle(indices[$0], indices[$1]) },
-    allowBrokenTransitivity: allowBrokenTransitivity)
+    allowBrokenTransitivity: allowBrokenTransitivity,
+    ${trace})
 }
 
 internal func _checkEquatableImpl<Instance : Equatable>(
@@ -1985,18 +1986,23 @@ internal func _checkEquatableImpl<Instance : Equatable>(
       let predictedXY = oracle(i, j)
       expectEqual(
         predictedXY, oracle(j, i),
-        "bad oracle: broken symmetry between indices \(i), \(j)")
+        "bad oracle: broken symmetry between indices \(i), \(j)",
+        stackTrace: ${stackTrace})
 
       let isEqualXY = x == y
       expectEqual(
         predictedXY, isEqualXY,
-        "lhs (at index \(i)): \(x)\nrhs (at index \(j)): \(y)",
+        (predictedXY
+           ? "expected equal, found not equal\n"
+           : "expected not equal, found equal\n") +
+        "lhs (at index \(i)): \(String(reflecting: x))\n" +
+        "rhs (at index \(j)): \(String(reflecting: y))",
         stackTrace: ${stackTrace})
 
       // Not-equal is an inverse of equal.
       expectNotEqual(
         isEqualXY, x != y,
-        "lhs (at index \(i)): \(x)\nrhs (at index \(j)): \(y)",
+        "lhs (at index \(i)): \(String(reflecting: x))\nrhs (at index \(j)): \(String(reflecting: y))",
         stackTrace: ${stackTrace})
 
       if !allowBrokenTransitivity {
@@ -2010,7 +2016,8 @@ internal func _checkEquatableImpl<Instance : Equatable>(
           for k in transitivityScoreboard[i].value {
             expectTrue(
               oracle(j, k),
-              "bad oracle: broken transitivity at indices \(i), \(j), \(k)")
+              "bad oracle: broken transitivity at indices \(i), \(j), \(k)",
+              stackTrace: ${stackTrace})
               // No need to check equality between actual values, we will check
               // them with the checks above.
           }
@@ -2147,11 +2154,28 @@ public func checkComparable<Instances : Collection>(
   for i in instances.indices {
     let x = instances[i]
 
-    expectFalse(x < x, ${trace})
-    expectFalse(x > x, ${trace})
-    expectTrue(x <= x, ${trace})
-    expectTrue(x >= x, ${trace})
-
+    expectFalse(
+      x < x,
+      "found 'x < x'\n" +
+      "at index \(i): \(String(reflecting: x))",
+      stackTrace: ${stackTrace})
+
+    expectFalse(
+      x > x,
+      "found 'x > x'\n" +
+      "at index \(i): \(String(reflecting: x))",
+      stackTrace: ${stackTrace})
+
+    expectTrue(x <= x,
+      "found 'x <= x' to be false\n" +
+      "at index \(i): \(String(reflecting: x))",
+      stackTrace: ${stackTrace})
+
+    expectTrue(x >= x,
+      "found 'x >= x' to be false\n" +
+      "at index \(i): \(String(reflecting: x))",
+      stackTrace: ${stackTrace})
+
     for j in instances.indices where i != j {
       let y = instances[j]
 
@@ -2163,11 +2187,30 @@ public func checkComparable<Instances : Collection>(
         + "(\(String(reflecting: i)), \(String(reflecting: j)))",
         stackTrace: ${stackTrace})
 
-      expectEqual(expected.isLT(), x < y, ${trace})
-      expectEqual(expected.isLE(), x <= y, ${trace})
-      expectEqual(expected.isGE(), x >= y, ${trace})
-      expectEqual(expected.isGT(), x > y, ${trace})
-
+      expectEqual(expected.isLT(), x < y,
+        "x < y\n" +
+        "lhs (at index \(i)): \(String(reflecting: x))\n" +
+        "rhs (at index \(j)): \(String(reflecting: y))",
+        stackTrace: ${stackTrace})
+
+      expectEqual(expected.isLE(), x <= y,
+        "x <= y\n" +
+        "lhs (at index \(i)): \(String(reflecting: x))\n" +
+        "rhs (at index \(j)): \(String(reflecting: y))",
+        stackTrace: ${stackTrace})
+
+      expectEqual(expected.isGE(), x >= y,
+        "x >= y\n" +
+        "lhs (at index \(i)): \(String(reflecting: x))\n" +
+        "rhs (at index \(j)): \(String(reflecting: y))",
+        stackTrace: ${stackTrace})
+
+      expectEqual(expected.isGT(), x > y,
+        "x > y\n" +
+        "lhs (at index \(i)): \(String(reflecting: x))\n" +
+        "rhs (at index \(j)): \(String(reflecting: y))",
+        stackTrace: ${stackTrace})
+
       for k in instances.indices {
         let expected2 = oracle(j, k)
         if expected == expected2 {

stdlib/public/core/StringComparable.swift

@@ -110,6 +110,7 @@ extension String {
 #if _runtime(_ObjC)
     // We only want to perform this optimization on objc runtimes. Elsewhere,
     // we will make it follow the unicode collation algorithm even for ASCII.
+    // This is consistent with Foundation, but incorrect as defined by Unicode.
     if _core.isASCII && rhs._core.isASCII {
       return _compareASCII(rhs)
     }
@@ -120,6 +121,10 @@ extension String {
 
 extension String : Equatable {
   public static func == (lhs: String, rhs: String) -> Bool {
+#if _runtime(_ObjC)
+    // We only want to perform this optimization on objc runtimes. Elsewhere,
+    // we will make it follow the unicode collation algorithm even for ASCII.
+    // This is consistent with Foundation, but incorrect as defined by Unicode.
     if lhs._core.isASCII && rhs._core.isASCII {
       if lhs._core.count != rhs._core.count {
         return false
@@ -128,6 +133,7 @@ extension String : Equatable {
         lhs._core.startASCII, rhs._core.startASCII,
         rhs._core.count) == 0
     }
+#endif
     return lhs._compareString(rhs) == 0
   }
 }

validation-test/stdlib/StringHashableComparable.swift.gyb

@@ -0,0 +1,97 @@
+// RUN: %target-run-simple-swiftgyb
+// REQUIRES: executable_test
+
+// This test requires that the standard library calls ICU
+// directly.  It is not specific to Linux, it is just that on
+// Apple platforms we are using the NSString bridge right now.
+
+// REQUIRES: OS=linux-gnu
+
+import StdlibUnittest
+import StdlibUnicodeUnittest
+
+func assertASCIIRepresentationIfPossible(_ s: String) {
+  for us in s.unicodeScalars {
+    if !us.isASCII {
+      return
+    }
+  }
+  precondition(s._core.isASCII)
+}
+
+func forceUTF16Representation(_ s: String) -> String {
+  var s = s
+  s += "\u{fffd}"
+  s.removeSubrange(s.index(before: s.endIndex)..<s.endIndex)
+  precondition(!s._core.isASCII)
+  return s
+}
+
+func calculateSortOrder(_ tests: inout [StringComparisonTest]) {
+  tests.sort {
+    collationElements(
+      $0.collationElements,
+      areLessThan: $1.collationElements
+    )
+  }
+
+  tests[0].order = 0
+  for i in tests.indices.dropFirst() {
+    if collationElements(
+      tests[i - 1].collationElements,
+      areLessThan: tests[i].collationElements
+    ) {
+      tests[i].order = tests[i - 1].order! + 1
+    } else {
+      tests[i].order = tests[i - 1].order!
+    }
+  }
+}
+
+func checkStringHashableComparable(
+  _ tests: [StringComparisonTest],
+  stackTrace: SourceLocStack = SourceLocStack(),
+  file: String = #file, line: UInt = #line
+) {
+  var tests = tests
+  calculateSortOrder(&tests)
+
+  func comparisonOracle(_ i: Int, _ j: Int) -> ExpectedComparisonResult {
+    return tests[i].order! <=> tests[j].order!
+  }
+
+  checkHashable(
+    tests.map { $0.string },
+    equalityOracle: { comparisonOracle($0, $1).isEQ() },
+    stackTrace: stackTrace.pushIf(true, file: file, line: line))
+
+  checkComparable(
+    tests.map { $0.string },
+    oracle: comparisonOracle,
+    stackTrace: stackTrace.pushIf(true, file: file, line: line))
+}
+
+var StringTests = TestSuite("StringTests")
+
+StringTests.test("StringComparisonTest.allTests: tests are in ASCII representation")
+  .forEach(in: StringComparisonTest.allTests) {
+  test in
+  assertASCIIRepresentationIfPossible(test.string)
+}
+
+StringTests.test("Comparable") {
+  let allTestsInUTF16Representation = StringComparisonTest.allTests.map {
+    test -> StringComparisonTest in
+    return StringComparisonTest(
+      forceUTF16Representation(test.string),
+      test.collationElements,
+      sourceLocation: SourceLoc(
+        test.loc.file,
+        test.loc.line,
+        comment: (test.loc.comment ?? "") + "\nin Unicode representation"))
+  }
+  checkStringHashableComparable(StringComparisonTest.allTests + allTestsInUTF16Representation)
+}
+
+runAllTests()
+

validation-test/stdlib/StringLowercasedUppercased.swift

@@ -0,0 +1,20 @@
+// RUN: %target-run-simple-swiftgyb
+// REQUIRES: executable_test
+
+// This test requires that the standard library calls ICU
+// directly.  It is not specific to Linux, it is just that on
+// Apple platforms we are using the NSString bridge right now.
+
+// REQUIRES: OS=linux-gnu
+
+import StdlibUnittest
+
+var StringTests = TestSuite("StringTests")
+
+StringTests.test("smoketest") {
+  expectEqual("i\u{0307}", "\u{0130}".lowercased())
+  expectEqual("SS", "\u{00df}".uppercased())
+}
+
+runAllTests()
+