Change threadDictionary to NSMutableDictionary #1762
Conversation
|
… yeah, it expresses as a NSMutableDictionary on Darwin as well. Thanks. |
|
@swift-ci please test and merge |
|
@omochi can you investigate the test failures? |
|
OK. I investigate. |
|
CI says this. I guess an error from this line. I am trying to reproduce test failure on Linux environment now. I may need few days. |
|
@omochi You may be running out of RAM in your docker container. Try using at least 2GB and try running with |
|
@spevans Thanks. Now I stopped another error. I try more. |
|
What command are you running to build swift? |
|
Sorry for late. I used this. |
|
It looks like you are building the debug versions which take up quite a lot of space. You could try rebuilding add in |
|
I think debug build is better to work on CI failure. |
|
@omochi If you have access to macOS and Xcode you may find it easier to develop/debug using that, just make sure to download the latest Xcode snapshot. |
|
In mac environment, my PR is no problem. By the way, I succeeded build now ! |
|
@swift-ci test |
|
I report progress. Issue is this essentially. import Foundation
class Cat {}
let a: Cat = Cat()
let array = NSMutableArray()
array.add(a as Any) |
omochi
force-pushed
the
tls-nsmutabledictionary
branch
from
6579490
to
aba7feb
Compare
|
I updated and fixed test failure. In my environment, there are other test failure about NSFileManager. |
|
Please test with the following pull request apple/swift-corelibs-xctest#240 |
|
@swift-ci test |
|
@millenomi Please review again |
|
That's a regression; any object should be storable in an array. I'll need to look into it. |
|
@swift-ci please test and merge |
|
why was the access control level changed? This is a backward incompatible change. |
|
Because I aligned it to Darwin's. https://developer.apple.com/documentation/foundation/thread/1411433-threaddictionary |
|
Yeah I guess the access control was wrong. But at least one open source library, RxSwift, depends on that bug. 😔 |
|
How does RxSwift on Darwin avoid problem? |
|
Yeah they could've avoided it. https://github.com/ReactiveX/RxSwift/blob/master/Platform/Platform.Linux.swift#L26 |
|
Ah, I understand. I read link and also They needed to split implementation for previous incompatible implementation between As a fact, this my PR broke backward compatibility. |
|
Another aspect of this: with This is not an abstract example because RxSwift actually does this. |
This PR changes type of
Thread.threadDictionarytoNSMutableDictionaryfromDictionary<String, Any>.The reasons are below.
Because apple's implementation for macOS/iOS is so.
A current implementation is hard to write code which can be shared between Linux and iOS.
I need to do for my project which has server side and client app.
Keywhich hasStringtype is hard to avoid conflict among some independent modules.Though I can do by giving prefix like domain,
but this needs more runtime access cost to test equality of key in dictionary.
With
NSMutableDictionary, I can useObjectIdentifierorUnsafeRawPointerof static key object.It is more safe from unexpected key conflict and cheap to test equality.
In original source, comment what about a implementation differs from Apple's is written.
But the reason of this is not written.
So I worry about some intention in it.
And if some reason about not able to use
NSMutableDictionary,I think that
Dictionary<AnyHashable, Any>is better for my 2nd reason.