[SR-2416] allow non-NSObject types to be archived #574

lhoward · 2016-08-19T04:18:56Z

This patch fixes a number of issues that were preventing non-NSObject types from being archived, even though the new method signatures imply that they can be (by taking Any).

fix regression introduced aa504dc where non-hashable objects could not be encoded
fix bug where non-NSObjects would be replaced with nil, causing a crash
remove explicit check that the encoded object is a NSObject
possibly should be a separate patch: transparently bridge value types to the corresponding reference type before encoding
tests for all over the above

parkera · 2016-10-06T17:59:03Z

I'm pretty sure you can't archive non-NSObject classes on Darwin either, actually.

parkera · 2016-10-06T17:59:09Z

@swift-ci please test

lhoward · 2016-10-07T15:26:12Z

On 6 Oct 2016, at 6:59 PM, Tony Parker [email protected] wrote:

I'm pretty sure you can't archive non-NSObject classes on Darwin either, actually.

Right, we discussed this earlier – this is for code portability because conforming to NSCoding without explicitly subclassing NSObject will automatically make a class @objc on Darwin (as I understand it), but not with SwiftFoundation.

parkera · 2016-10-07T16:58:14Z

Yes, thanks for the reminder.

parkera · 2016-10-07T16:58:39Z

Build failure:

TestFoundation/TestNSKeyedArchiver.swift:156:63: error: 'Any' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
        return test_archive(object, classes: [type(of: object as AnyObject)], allowsSecureCoding: allowsSecureCoding)
                                                       ~~~~~~~^~~~~~~~~~~~
                                                              as!
ninja: build stopped: subcommand failed.

lhoward · 2016-10-07T19:49:46Z

It builds fine with the XCode 8.0 toolchain, I’ll try with a recent snapshot and fix any errors.

parkera · 2016-10-07T19:56:55Z

It's probably a cross-platform problem with bridging on Linux; I don't think building in Xcode will show the issue.

lhoward · 2016-10-07T19:58:52Z

OK, should I just make the change suggested by the compiler below then? I can’t test on Linux right now (just off a 30 hour flight).

On 7 Oct 2016, at 6:58 PM, Tony Parker [email protected] wrote:

Build failure:

TestFoundation/TestNSKeyedArchiver.swift:156:63: error: 'Any' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
return test_archive(object, classes: [type(of: object as AnyObject)], allowsSecureCoding: allowsSecureCoding)
~~~~~~~^~~~~~~~~~~~
as!
ninja: build stopped: subcommand failed.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #574 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AAhCNwoEAA5fDkqTjpi0FLEWIDqOySpsks5qxnpHgaJpZM4JoIha.

www.lukehoward.com
soundcloud.com/lukehoward

parkera · 2016-10-07T20:14:10Z

I think that'll just end up creating a warning on Mac. @phausler what's the right incantation here?

lhoward · 2016-10-07T20:18:50Z

It does create a warning on Mac :(

Sent from my iPhone

On 7 Oct. 2016, at 22:14, Tony Parker [email protected] wrote:

I think that'll just end up creating a warning on Mac. @phausler what's the right incantation here?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

phausler · 2016-10-07T20:25:16Z

TestFoundation/TestNSKeyedArchiver.swift

        // test both XML and binary encodings
        test_archive(object, classes: classes, allowsSecureCoding: allowsSecureCoding, outputFormat: PropertyListSerialization.PropertyListFormat.xml)
        test_archive(object, classes: classes, allowsSecureCoding: allowsSecureCoding, outputFormat: PropertyListSerialization.PropertyListFormat.binary)
    }

-    private func test_archive(_ object: NSObject, allowsSecureCoding: Bool = true) {
-        return test_archive(object, classes: [type(of: object)], allowsSecureCoding: allowsSecureCoding)
+    private func test_archive(_ object: Any, allowsSecureCoding: Bool = true) {


This should probably be NSObject or AnyObject still.

This patch fixes a number of issues that were preventing non-NSObject types from being archived, even though the new method signatures imply that they can be (by taking Any). * fix regression introduced aa504dc where non-hashable objects could not be encoded * fix bug where non-NSObjects would be replaced with nil, causing a crash * remove explicit check that the encoded object is a NSObject * possibly should be a separate patch: transparently bridge value types to the corresponding reference type before encoding * tests for all over the above

lhoward · 2016-10-08T06:08:34Z

Fixed.

On 7 Oct 2016, at 10:25 PM, Philippe Hausler [email protected] wrote:

@phausler commented on this pull request.

In TestFoundation/TestNSKeyedArchiver.swift #574 (review):
     // test both XML and binary encodings
     test_archive(object, classes: classes, allowsSecureCoding: allowsSecureCoding, outputFormat: PropertyListSerialization.PropertyListFormat.xml)
     test_archive(object, classes: classes, allowsSecureCoding: allowsSecureCoding, outputFormat: PropertyListSerialization.PropertyListFormat.binary)
 }
private func test_archive(_ object: NSObject, allowsSecureCoding: Bool = true) {
   return test_archive(object, classes: [type(of: object)], allowsSecureCoding: allowsSecureCoding)
private func test_archive(_ object: Any, allowsSecureCoding: Bool = true) {
This should probably be NSObject or AnyObject still.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #574 (review), or mute the thread https://github.com/notifications/unsubscribe-auth/AAhCN1I7vjO0t2jx9mvb9DMxfzQ4jTtyks5qxqq0gaJpZM4JoIha.

www.lukehoward.com
soundcloud.com/lukehoward

parkera · 2016-10-10T16:36:44Z

@swift-ci please test

lhoward · 2016-10-11T06:39:00Z

This build failure doesn't seem to be SwiftFoundation related?

parkera · 2016-10-11T16:36:26Z

Yah, let's try that again.

parkera · 2016-10-11T16:36:30Z

@swift-ci please test

parkera · 2016-10-27T16:06:18Z

@swift-ci please test and merge

[pull] swiftwasm from main

lhoward mentioned this pull request Aug 19, 2016

[SR-2384] remove NSUniqueObject #563

Merged

lhoward force-pushed the SR-2416 branch 4 times, most recently from 26104f2 to da3d6ef Compare August 25, 2016 06:04

lhoward force-pushed the SR-2416 branch from da3d6ef to b06f636 Compare August 31, 2016 02:22

phausler reviewed Oct 7, 2016

View reviewed changes

lhoward force-pushed the SR-2416 branch from b06f636 to ec3f61e Compare October 8, 2016 06:08

swift-ci merged commit 762f0c0 into swiftlang:master Oct 27, 2016

kateinoigakukun pushed a commit to kateinoigakukun/swift-corelibs-foundation that referenced this pull request Oct 11, 2023

Merge pull request swiftlang#574 from swiftwasm/main

fe6b8aa

[pull] swiftwasm from main

[SR-2416] allow non-NSObject types to be archived #574

[SR-2416] allow non-NSObject types to be archived #574

Uh oh!

Conversation

lhoward commented Aug 19, 2016

Uh oh!

parkera commented Oct 6, 2016

Uh oh!

parkera commented Oct 6, 2016

Uh oh!

lhoward commented Oct 7, 2016

Uh oh!

parkera commented Oct 7, 2016

Uh oh!

parkera commented Oct 7, 2016

Uh oh!

lhoward commented Oct 7, 2016

Uh oh!

parkera commented Oct 7, 2016

Uh oh!

lhoward commented Oct 7, 2016

Uh oh!

parkera commented Oct 7, 2016

Uh oh!

lhoward commented Oct 7, 2016

Uh oh!

phausler Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

lhoward commented Oct 8, 2016

Uh oh!

parkera commented Oct 10, 2016

Uh oh!

lhoward commented Oct 11, 2016

Uh oh!

parkera commented Oct 11, 2016

Uh oh!

parkera commented Oct 11, 2016

Uh oh!

parkera commented Oct 27, 2016

Uh oh!

Uh oh!