[String.Index] Obsolete encodedOffset var/init #22108

milseman · 2019-01-25T04:20:56Z

[String.Index] Deprecate encodedOffset var/init

String.Index has an encodedOffset-based initializer and computed
property that exists for serialization purposes. It was documented as
UTF-16 in the SE proposal introducing it, which was String's
underlying encoding at the time, but the dream of String even then was
to abstract away whatever encoding happend to be used.

Serialization needs an explicit encoding for serialized indices to
make sense: the offsets need to align with the view. With String
utilizing UTF-8 encoding for native contents in Swift 5, serialization
isn't necessarily the most efficient in UTF-16.

Furthermore, the majority of usage of encodedOffset in the wild is
buggy and operates under the assumption that a UTF-16 code unit
was a Swift Character, which isn't even valid if the String is known
to be all-ASCII (because CR-LF).

This change introduces a set of new initializers and methods with an
explicit encoding (through the type of the provided view) for
serialization. It also adds an init and method for String's default
view, which most code in the wild should migrate to if it eliminates
their underlying bugs.

Resolves SR-9749. Much thanks to @norio-nomura for catching this!

rdar://problem/47605131

airspeedswift · 2019-01-28T01:10:40Z

@swift-ci please smoke test

milseman · 2019-01-28T04:27:18Z

@swift-ci please test

swift-ci · 2019-01-28T05:30:53Z

Build failed
Swift Test OS X Platform
Git Sha - 0cff8fb

milseman · 2019-01-28T22:23:19Z

@swift-ci please test

swift-ci · 2019-01-28T22:25:40Z

Build failed
Swift Test Linux Platform
Git Sha - 0cff8fb

swift-ci · 2019-01-28T22:33:10Z

Build failed
Swift Test OS X Platform
Git Sha - 0cff8fb

String.Index has an encodedOffset-based initializer and computed property that exists for serialization purposes. It was documented as UTF-16 in the SE proposal introducing it, which was String's underlying encoding at the time, but the dream of String even then was to abstract away whatever encoding happend to be used. Serialization needs an explicit encoding for serialized indices to make sense: the offsets need to align with the view. With String utilizing UTF-8 encoding for native contents in Swift 5, serialization isn't necessarily the most efficient in UTF-16. Furthermore, the majority of usage of encodedOffset in the wild is buggy and operates under the assumption that a UTF-16 code unit was a Swift Character, which isn't even valid if the String is known to be all-ASCII (because CR-LF). This change introduces a pair of semantics-preserving alternatives to encodedOffset that explicitly call out the UTF-16 assumption. These serve as a gentle off-ramp for current mis-uses of encodedOffset.

tkremenek · 2019-02-15T20:46:52Z

This proposal has been accepted and can be merged.

milseman · 2019-02-15T23:00:39Z

@swift-ci please test

swift-ci · 2019-02-16T00:32:55Z

Build failed
Swift Test Linux Platform
Git Sha - 415cc8f

milseman · 2019-02-16T01:05:08Z

Linux Failures seems unrelated:

18:32:51 
<EXPR>:0: error: TestProcess.test_passthrough_environment : threw error "The operation could not be completed"

milseman · 2019-02-16T01:05:19Z

@swift-ci please test Linux Platform

benrimmington · 2019-02-16T02:03:59Z

stdlib/public/core/StringIndex.swift

+  @available(swift, deprecated: 4.2, message: """
+    encodedOffset has been deprecated as most common usage is incorrect. \
+    Use utf16Offset(in:) to achieve the same behavior.
+    """)


Should the initializer and property be deprecated: 4.2 or deprecated: 5.0?

Should the deprecated APIs be moved to MigrationSupport.swift if possible?

A multi-line message (even with escaped line breaks) won't display properly in Xcode 10.1 (and perhaps in Xcode 10.2 beta).

I believe that availability matches the language version and not deployment target. This should be deprecated regardless of what is passed in -swift-version. @jrose-apple / @moiseev what do you think?

I think MigrationSupport is done for obsoleted. We should probably obsolete this around 5.1 or so and move it then.

I'll have to try that then.

MigrationSupport has some non-obsoleted APIs:

DictionaryLiteral

UTF8, UTF16, UTF32, UnicodeScalar (which are also non-deprecated).

_boundsCheck(_:) (I'm not sure what this is doing here, it seems to be unused).

index(where:), index(of:)

swift-ci · 2019-02-16T02:22:31Z

Build failed
Swift Test Linux Platform
Git Sha - 415cc8f

milseman · 2019-02-17T00:24:16Z

Linux PR bot has been failing in multiple PRs. Trying again

milseman · 2019-02-17T00:24:23Z

@swift-ci please test linux platform

swift-ci · 2019-02-17T01:35:16Z

Build failed
Swift Test Linux Platform
Git Sha - 415cc8f

tkremenek · 2019-02-17T18:14:00Z

@swift-ci clean test

swift-ci · 2019-02-17T18:50:39Z

Build failed
Swift Test Linux Platform
Git Sha - 415cc8f

milseman · 2019-02-18T18:20:33Z

@swift-ci please test linux platform

milseman · 2019-02-18T22:42:46Z

@swift-ci please test linux platform

swift-ci · 2019-02-18T23:55:20Z

Build failed
Swift Test Linux Platform
Git Sha - 415cc8f

milseman · 2019-02-19T18:50:02Z

Linux failure is unrelated and appearing on other PRs:
<EXPR>:0: error: TestProcess.test_passthrough_environment : threw error "The operation could not be completed"

milseman · 2019-02-19T18:50:10Z

@swift-ci please test linux platform

milseman requested a review from airspeedswift January 25, 2019 04:20

milseman changed the title ~~[String.Index] Deprecate/obsolete encodedOffset var/init~~ [WIP DO NOT MERGE][String.Index] Deprecate/obsolete encodedOffset var/init Jan 25, 2019

norio-nomura mentioned this pull request Jan 25, 2019

SR-9454: StringProtocol.lineRange(for:) returns invalid range on Linux swiftlang/swift-corelibs-foundation#1841

Merged

milseman force-pushed the en_gadus_offset branch 2 times, most recently from dd38b3b to 0cff8fb Compare January 26, 2019 01:29

milseman changed the title ~~[WIP DO NOT MERGE][String.Index] Deprecate/obsolete encodedOffset var/init~~ [WIP DO NOT MERGE][String.Index] Obsolete encodedOffset var/init Jan 29, 2019

milseman force-pushed the en_gadus_offset branch from b596724 to e210805 Compare February 14, 2019 02:40

milseman changed the title ~~[WIP DO NOT MERGE][String.Index] Obsolete encodedOffset var/init~~ [String.Index] Obsolete encodedOffset var/init Feb 14, 2019

milseman force-pushed the en_gadus_offset branch from e210805 to 415cc8f Compare February 14, 2019 02:42

benrimmington reviewed Feb 16, 2019

View reviewed changes

milseman mentioned this pull request Feb 18, 2019

[5.0][String.Index] Deprecate encodedOffset var/init #22695

Merged

milseman merged commit f879607 into swiftlang:master Feb 19, 2019

milseman deleted the en_gadus_offset branch February 19, 2019 22:43

milseman mannequin mentioned this pull request Jan 25, 2019

[SR-9749] The encodedOffset of String.Index has been changed to either UTF-8 or UTF-16. #52180

Closed

[String.Index] Obsolete encodedOffset var/init #22108

[String.Index] Obsolete encodedOffset var/init #22108

Uh oh!

Conversation

milseman commented Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

airspeedswift commented Jan 28, 2019

Uh oh!

milseman commented Jan 28, 2019

Uh oh!

swift-ci commented Jan 28, 2019

Uh oh!

milseman commented Jan 28, 2019

Uh oh!

swift-ci commented Jan 28, 2019

Uh oh!

swift-ci commented Jan 28, 2019

Uh oh!

tkremenek commented Feb 15, 2019

Uh oh!

milseman commented Feb 15, 2019

Uh oh!

swift-ci commented Feb 16, 2019

Uh oh!

milseman commented Feb 16, 2019

Uh oh!

milseman commented Feb 16, 2019

Uh oh!

benrimmington Feb 16, 2019

Choose a reason for hiding this comment

Uh oh!

milseman Feb 17, 2019

Choose a reason for hiding this comment

Uh oh!

benrimmington Feb 17, 2019

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Feb 16, 2019

Uh oh!

milseman commented Feb 17, 2019

Uh oh!

milseman commented Feb 17, 2019

Uh oh!

swift-ci commented Feb 17, 2019

Uh oh!

tkremenek commented Feb 17, 2019

Uh oh!

swift-ci commented Feb 17, 2019

Uh oh!

milseman commented Feb 18, 2019

Uh oh!

milseman commented Feb 18, 2019

Uh oh!

swift-ci commented Feb 18, 2019

Uh oh!

milseman commented Feb 19, 2019

Uh oh!

milseman commented Feb 19, 2019

Uh oh!

Uh oh!

milseman commented Jan 25, 2019 •

edited

Loading