[SR-10858] Bridging NSString-keyed Dictionaries should only set allowsDuplicates if there are non-ASCII (non-NFC?) Strings

[Catfish-Man]

Previous ID	SR-10858
Radar	None
Original Reporter	@Catfish-Man
Type	Improvement
Status	In Progress
Resolution

Additional Detail from JIRA

Votes	1
Component/s	Standard Library
Labels	Improvement, StarterBug
Assignee	valeriyvan (JIRA)
Priority	Medium

md5: d18df56f9579713f39cb8ff948cf30ca

Issue Description:

Currently we have code like this:

// String and NSString have different concepts of equality, so
// string-keyed NSDictionaries may generate key collisions when bridged
// over to Swift. See rdar://problem/35995647
let handleDuplicates = (Key.self == String.self)
      
result = Dictionary(_unsafeUninitializedCapacity: numElems,
   allowingDuplicates: handleDuplicates) { keys, vals in
        

This is overly conservative. We only actually need to set allowingDuplicates if there are non-NFC NSStrings!

[swift-ci]

Comment by Tim Searle (JIRA)

Hey, I'm interested in taking a look at this as a potentially first contribution to the standard library. What would be the correct (and efficient way) to check if the `String` contained non-NFC characters? Would the check itself outweigh any benefits to not setting `allowingDuplicates` for all `Dictionary`'s with `String` keys?

Also, where would I find the test cases for this part of the library? Would be good to confirm whether or not the existing tests already cover this case to detect any regressions a change might make.

[Catfish-Man]

Awesome, thanks for taking a look 🙂

I’m on my phone at the moment, so don’t have the source handy to reference, but a simple subset of this that’s easy to check is all ASCII Strings are automatically NFC and there’s a fast isASCII flag on _StringGuts.

The simplest way to find the relevant tests is probably just to always set allowingDuplicates to false and then see which tests break.

[swift-ci]

Comment by Tim Searle (JIRA)

Apologies for the delay! Only just got around to getting my Swift development environment setup. Quick question, I built the Swift source for Xcode, with the debug flag for the stdlib. Would that make the correct command to run the tests afterwards `swift/utils/build-script --release-debuginfo --debug-swift-stdlib -x -t` ? Also interested to know what sort of workflow/environments others are using? As building and running tests will have a fair bit more overhead than general application development.

Pretty new to this! So I am still trying to figure out what my workflow will look like. 🙂 Pretty sure once I have this sorted, getting an initial PR won't be too much effort.

[Catfish-Man]

test comment

[Catfish-Man]

No worries, and no hurry. That command line looks about right except I'd leave the -x off once you have the project generated. Workflow for the stdlib is… well, I suggest patience… you won't have fully working tooling support (so only partial autocomplete or jump to definition at best, for example), and builds take a while. I use 'git grep' a lot to find things. Sometimes it can be worth yanking some code out into a standalone project to play with it in a less heavyweight environment.

[belkadan]

Still working on this, tim_ (JIRA User)?

(I'm checking in on all the StarterBugs that haven't been touched in over a month; it's totally fine if you just haven't had time but still want to keep it.)

[swift-ci]

Comment by Tim Searle (JIRA)

Hey @belkadan, disappointingly I haven't had the opportunity to make as much progress as I'd like (beyond getting my environment up and running) and the next few weeks are quite busy for me - so for now I'll unassign the ticket in case anyone else wants to pick it up, thanks for the reminder!

[k-thorat]

Hey @Catfish-Man Is this still valid? I don't see any references to _unsafeUninitializedCapacity

Thank you!

[Catfish-Man]

The situation got complicated for a bit. I think it may become valid again in the future, but for the moment it's not, sorry about the confusion.