Skip to content

Uncomment NSUserDefaults dictionaryRepresentation #1086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CoreFoundation/Preferences.subproj/CFPreferences.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ keysToFetch that are not present in the returned dictionary
are not present in the domain. If keysToFetch is NULL, all
keys are fetched. */
CF_EXPORT
CFDictionaryRef CFPreferencesCopyMultiple(_Nullable CFArrayRef keysToFetch, CFStringRef applicationID, CFStringRef userName, CFStringRef hostName);
_Nullable CFDictionaryRef CFPreferencesCopyMultiple(_Nullable CFArrayRef keysToFetch, CFStringRef applicationID, CFStringRef userName, CFStringRef hostName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have any other nullable annotations in CF headers. Can we just treat this as an IUO in Swift instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That function does return NULL though and that header file has other _Nullable annotations, with a default of CF_ASSUME_NONNULL_BEGIN. So I don't see a problem with this change and it does fix an annoying warning.

Is it not a good idea to annotate as many of the function prototypes as possible over time? Or is there some downside to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this annotation, Swift will reject (aPref._swiftObject) as? [NSString: Any] because it doesn't think _swiftObject is optional.

I had my doubt then I found functions like CFPreferencesCopyApplicationList and CFPreferencesCopyKeyList below actually has _Nullable annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides, as @spevans pointed out, it does return NULL in some condition. And the changes seems to work fine on both Linux & macOS. The only concern I have is the possibility of conflict if there will be any merging between the CF we used here & the CF in macOS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think we'll just have to figure out what to do with this when we merge with Darwin.


/* The primitive set function; all arguments except value must be
non-NULL. If value is NULL, the given key is removed */
Expand Down
6 changes: 1 addition & 5 deletions Foundation/NSUserDefaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,8 @@ open class UserDefaults: NSObject {
}

open func dictionaryRepresentation() -> [String : Any] {
NSUnimplemented()
/*
Currently crashes the compiler.
guard let aPref = CFPreferencesCopyMultiple(nil, kCFPreferencesCurrentApplication, kCFPreferencesCurrentUser, kCFPreferencesCurrentHost),
bPref = (aPref._swiftObject) as? [NSString: Any] else {
let bPref = (aPref._swiftObject) as? [NSString: Any] else {
return registeredDefaults
}
var allDefaults = registeredDefaults
Expand All @@ -253,7 +250,6 @@ open class UserDefaults: NSObject {
}

return allDefaults
*/
}

open var volatileDomainNames: [String] { NSUnimplemented() }
Expand Down