Re: [swift-dev] String comparison improvements

2018-01-17 Thread Itai Ferber via swift-dev

Hi Lance (and Michael — I’ll keep the conversation in one thread),

Sounds reasonable. My one concern is that behavior around implicitly 
bridged strings is going to change, in a potentially problematic and 
"magic" way:


```swift
let s1: String = …
let s2: String = …

let mySet = NSMutableSet()
mySet.add(s1)
mySet.add(s2)

// s1 != s2 by Swift ordering rules, but CFStringCompare(s1, s2, …) == 
kCFCompareEqualTo

print(mySet.count) // => 1 
```

or alternatively:

```swift
let s1: String = …
let s2: String = …

let mySet = NSMutableSet()
mySet.add(s1)

// s1 == s2 by Swift ordering rules, but CFStringCompare(s1, s2, …) != 
kCFCompareEqualTo

print(mySet.contains(s2)) // => false 
```

I don’t think there’s much we can do about this, but this is 
something we’re going to have to watch out for — this will be a 
relatively rare and very subtle behavior change.


— Itai

On 17 Jan 2018, at 13:56, Lance Parker wrote:


Comments inline below


On Jan 17, 2018, at 1:46 PM, Itai Ferber  wrote:

Hi Lance,

I read Michael’s emails but I don’t remember at the moment — 
what is the new string comparison implementation going to be based 
on?


The new approach uses the lexicographical ordering of NFC-normalized 
UTF-16 code units. For two known ASCII strings, we just use memcmp.
Also, how will this affect bridged strings? If I compare two 
NSStrings, I may get a different result than if I compare the same 
two strings as bridged through String, correct?


If I understand correctly, you’re asking what will happen if you 
have two strings explicitly typed as NSString in swift and you compare 
them. I believe they’ll still use whatever NSString does for 
comparison today, so CFStringCompare. For Swift strings backed by a 
bridged NSString, this new comparison method will be used.


It might make sense for explicit NSStrings in Swift to use the new 
method as well. What do you think?

— Itai

On 17 Jan 2018, at 13:19, Lance Parker via swift-dev wrote:

Hey Swift-Dev,

The swift standard library team  have been working on a new 
implementation for comparing Swift strings for Swift 5. Michael 
touched on the motivations in the State of String email but I’ll 
summarize here:


The Swift String comparison implementations on Apple platforms and 
Linux differ in results and performance. Apple platforms use 
CFStringCompare with no locale, while Linux uses ICU libraries. 
Unifying the algorithms that Swift strings use for comparison is 
reason alone for doing a new implementation.
We've come up with some great common fast paths that speed up 
comparisons for a lot of common cases. Our microbenchmarks show up to 
a 6.8x increase in performance and there is still some low hanging 
fruit in our implementation that would bring further speedups.


Bare in mind this is not intended to be a replacement for sorting 
strings that will be presented to users, for that developers should 
stick to NSLocalizedString APIs.

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev 




___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] String comparison improvements

2018-01-17 Thread Itai Ferber via swift-dev

Hi Lance,

I read Michael’s emails but I don’t remember at the moment — what 
is the new string comparison implementation going to be based on?
Also, how will this affect bridged strings? If I compare two 
`NSString`s, I may get a different result than if I compare the same two 
strings as bridged through `String`, correct?


— Itai

On 17 Jan 2018, at 13:19, Lance Parker via swift-dev wrote:


Hey Swift-Dev,

The swift standard library team  have been working on a new 
implementation for comparing Swift strings for Swift 5. Michael 
touched on the motivations in the State of String email but I’ll 
summarize here:


The Swift String comparison implementations on Apple platforms and 
Linux differ in results and performance. Apple platforms use 
CFStringCompare with no locale, while Linux uses ICU libraries. 
Unifying the algorithms that Swift strings use for comparison is 
reason alone for doing a new implementation.
We've come up with some great common fast paths that speed up 
comparisons for a lot of common cases. Our microbenchmarks show up to 
a 6.8x increase in performance and there is still some low hanging 
fruit in our implementation that would bring further speedups.


Bare in mind this is not intended to be a replacement for sorting 
strings that will be presented to users, for that developers should 
stick to NSLocalizedString APIs.




___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] [Swift CI] Build Failure: OSS - Swift Package - OS X (master) #1021

2018-01-11 Thread Itai Ferber via swift-dev

Hi Doug, Slava,

Looks like this is still an issue — just hit this on 
https://github.com/apple/swift/pull/13879

Does this failure look familiar to either of you?

— Itai

On 11 Jan 2018, at 7:12, swift-ci--- via swift-dev wrote:


##

# [FAILURE] oss-swift-package-osx [#1021]

Build URL: | 
---|---
Project: | oss-swift-package-osx
Date of build: | Thu, 11 Jan 2018 06:39:53 -0600
Build duration: | 2 hr 32 min

## Identified problems:

  * Regression test failed: This build failed because a regression 
test in the test suite FAILed. Below is a list of all errors:
* [Indication 
1]()
  * Compile Error: This build failed because of a compile error. Below 
is a list of all errors in the build log:
* [Indication 
1]()





## Changes

  * Commit **005267f2b087f1db34688670b5e273015dcdd2f2** by 
**spestov:**


SIL: Remove obsolete comments from FormalLinkage.h

* **edit** : include/swift/SIL/FormalLinkage.h
  *

  * Commit **3dae007465eca11d6d202ed411579ffc86826f2a** by 
**spestov:**


IRGen: Remove FieldAccess::NonConstantIndirect

* **edit** : lib/IRGen/GenClass.cpp
* **edit** : lib/IRGen/GenDecl.cpp
* **edit** : lib/IRGen/GenMeta.cpp
* **edit** : lib/TBDGen/TBDGen.cpp
* **edit** : lib/IRGen/IRGenModule.h
* **edit** : lib/IRGen/IRGenMangler.h
* **edit** : lib/IRGen/Linking.cpp
* **edit** : include/swift/IRGen/Linking.h
* **edit** : lib/IRGen/StructLayout.h
* **edit** : lib/IRGen/GenKeyPath.cpp
  *

  * Commit **d134529e5491cce10b3af76e98356952e2c472f9** by 
**spestov:**


IRGen: Field offset symbols of resilient classes have hidden linkage

* **edit** : test/IRGen/class_resilience.swift
* **edit** : lib/IRGen/GenDecl.cpp
  *

  * Commit **065d18921f57d8e18637a6b9dd6d152806e5ed55** by 
**spestov:**


IRGen: Test protocol dispatch thunk for a throwing method requirement

* **edit** : test/IRGen/protocol_resilience_thunks.swift
  *

  * Commit **df10c8659b19ea0bf6ff34d95c87db4e3b4b480a** by 
**spestov:**


SILOptimizer: Fix mandatory inlining bug with resilience

* **edit** : lib/SILOptimizer/Mandatory/MandatoryInlining.cpp
* **add** : test/SILOptimizer/mandatory_inlining_resilience.sil
  *

  * Commit **992da7494ba8a0086f34e09b9dbd61ca6fb46d5f** by 
**spestov:**


SIL: Remove some uses of LValueType

* **edit** : lib/SIL/TypeLowering.cpp
  *

  * Commit **43aebc832e0a9c596d89a77c65b894b8ac570c95** by **github:**

Revert "IUO: Add support for non-subscript dynamic lookup."

* **edit** : lib/Sema/ConstraintLocator.h
* **edit** : lib/Sema/ConstraintSystem.cpp
* **edit** : lib/Sema/CSApply.cpp
* **edit** : lib/Sema/ConstraintSystem.h
* **edit** : lib/Sema/ConstraintLocator.cpp
  *

  * Commit **2c86f918def205b9414d89ee7967360ef56fc88f** by 
**dgregor:**


[Runtime] Add tests for demanling never-before-seen types to metadata.

* **edit** : test/Runtime/demangleToMetadata.swift
  *

  * Commit **35268902332dd3ac0b746f863c9517bc2a96c1cb** by 
**spestov:**


TypeReconstruction: Fix reconstruction of InOutType

* **edit** : test/IDE/reconstruct_type_from_mangled_name.swift
* **edit** : lib/IDE/TypeReconstruction.cpp
  *

  * Commit **759f4c3a2cb1531958465058849fabdb4e09e628** by 
**dgregor:**


[Runtime] Skip Objective-C type records when looking for types.

* **edit** : test/Runtime/demangleToMetadataObjC.swift
* **edit** : stdlib/public/runtime/ProtocolConformance.cpp
* **edit** : stdlib/public/runtime/MetadataLookup.cpp
  *

  * Commit **d104a7aec405177384a4cf92a101b99297183fed** by 
**spestov:**


[Swift] Update for mangler API change

* **edit** : source/Symbol/SwiftASTContext.cpp
  *




___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev
___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


Re: [swift-dev] Conditional conformance: Removing the opt-in flag

2018-01-02 Thread Itai Ferber via swift-dev



On 27 Dec 2017, at 12:41, Douglas Gregor via swift-dev wrote:


Sent from my iPhone


On Dec 27, 2017, at 11:05 AM, Karl Wagner  wrote:



On 22. Dec 2017, at 07:13, Ted Kremenek via swift-dev 
 wrote:




On Dec 19, 2017, at 9:39 PM, Ted Kremenek via swift-dev 
 wrote:




On Dec 19, 2017, at 8:57 PM, Douglas Gregor  
wrote:




Sent from my iPhone

On Dec 19, 2017, at 8:31 PM, Ted Kremenek  
wrote:




On Dec 19, 2017, at 3:59 PM, Douglas Gregor  
wrote:




On Dec 19, 2017, at 2:26 PM, Ted Kremenek via swift-dev 
 wrote:



On Dec 18, 2017, 4:53 PM -0800, Douglas Gregor via swift-dev 
, wrote:

Hi all,

A little while back, I added an error to the Swift 4.1 
compiler that complains if one tries to use conditional 
conformances, along with a flag 
“-enable-experimental-conditional-conformances” to enable 
the feature. We did this because we haven’t implemented the 
complete proposal yet; specifically, we don’t yet handle 
dynamic casting that involves conditional conformances, and 
won’t in Swift 4.1.


I’d like to take away the 
"-enable-experimental-conditional-conformances” flag and 
always allow conditional conformances in Swift 4.1, because 
the changes in the standard library that make use of 
conditional conformances can force users to change their code 
*to themselves use conditional conformances*. Specifically, if 
they had code like this:


extension MutableSlice : P { }
extension MutableBidirectionalSlice : P { }
// …

they’ll get an error about overlapping conformances, and 
need to do something like the following to fix the issue:


extension Slice: P where Base: MutableCollection { }

which is way more elegant, but would require passing 
"-enable-experimental-conditional-conformances”. That 
seems… unfortunate… given that we’re forcing them to use 
this feature.


My proposal is, specifically:

Allow conditional conformances to be used in Swift 4.1 (no 
flag required)
Drop the -enable-experimental-conditional-conformances flag 
entirely
Add a runtime warning when an attempt to dynamic cast fails 
due to a conditional conformance, so at least users know 
what’s going on




The last bullet doesn’t feel right to me.  It sounds like we 
would ship a feature that we know only partially works, but 
issue a runtime warning in the case we know isn’t fully 
implemented?  I’m I interpretting that point correctly?


Yes, that’s correct. We will fail to match the conformance 
(i.e., return “nil” from an “as?” cast), which might be 
correct and might be wrong.


- Doug


Hmm.  I’m concerned that a warning runtime would be to settle. 
Many people would possibly not even notice it.  It’s 
essentially an edge case in a feature that isn’t fully 
implemented and thus that part of the feature should not be used 
yet.


What do you think about making this a hard runtime error instead, 
similar to how we are approaching runtime issues for exclusivity 
checking?  That would be impossible to miss and would convey the 
optics that this runtime aspect of the feature is not yet 
supported and thus should not be used.


I’d rather not make it a runtime error, because code that’s 
doing dynamic casting to a protocol is generally already handling 
the “nil” case (“as?” syntax), so aborting the program 
feels far too strong.


  - Doug


For me I think the part I’m struggling with is that making it a 
warning conflates two things together: expected failure in the 
dynamic cast because the value you are casting doesn’t have that 
type or — in this case — failure because the cast can never 
succeed because it is not supported yet.  I feel like we would be 
silently swallowing an unsupported condition.  If that didn’t 
matter, why bother issuing a warning?  Clearly were trying to send 
some kind of message here about this not being supported.


Doug and I chatted a bit offline.

I’m now more on the side of thinking a warning is a reasonable 
approach.  I’m still concerned that it will be unnoticed by some 
developers, and I am mixed on conflating failure the cast of “this 
doesn’t work at all for this specific type because it has a 
conditional conformance” versus “this didn’t work because the 
type didn’t conform to the protocol”.  That said, I think the 
cases impacted here are likely very, very small — and a crash in 
the program is probably excessive.

___
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev



What about if we disabled conditional conformances for non-generic 
protocols (or keep that part behind the flag)? It seems a bit 
arbitrary, but IIRC, the standard library uses conditional 
conformances for things like Equatable and the various faces of 
Collection, which are not runtime-castable anyway.


This is a reasonable approach. To make it