TimeZone, Calendar, and Locale disagree on how .current is serialized
#1491
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test |
itingliu
approved these changes
Sep 4, 2025
| return result as CFDictionary | ||
| } | ||
| var icuSymbolsAndStrings = try container.decodeIfPresent(ICUSymbolsAndStrings.self, forKey: .icuSymbolsAndStrings) | ||
| if (icuDateFormats != nil || icuNumberSymbols != nil) && icuSymbolsAndStrings == nil { |
There was a problem hiding this comment.
Nit: It looks like we can always create a icuSymbolsAndStrings since we're going to always need one below at init?
| self.icuTimeFormatStrings = try container.decodeIfPresent([String : String].self, forKey: .icuTimeFormatStrings).map { $0 as CFDictionary } | ||
| self.icuNumberFormatStrings = try container.decodeIfPresent([String : String].self, forKey: .icuNumberFormatStrings).map { $0 as CFDictionary } | ||
|
|
||
| // Will be filled in by LocalePreferences serialized value |
There was a problem hiding this comment.
Thanks for this comment. I was indeed confused above why these two are handled separately from the others.
|
@swift-ci please test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's been a handful of changes over the years in this area, but the situation we have today is that
TimeZone,Calendar, andLocaleall have different behaviors for how they encode.current. All 3 mostly agree that.autoupdatingCurrentis always encoded as as sentinel value becoming the.autoupdatingCurrentat decode time, and that fixed values are decoded simply as fixed values (Calendar doesn't do the latter sometimes). However:TimeZonealways encodes.currentas a fixed value. When decoded it is the same time zone that was encoded regardless of.currentat decode timeLocalealways encodes.currentas a sentinel value. When decoded, it takes the value of whatever.currentis at decode time regardless of the locale that was encodedCalendarencodes a sentinel current value if the calendar to be encoded is equivalent to the current locale (i.e. if it has the same identifier/properties). This means that.currentis always encoded as a sentinel value likeLocale, but some fixed values are also encoded as a.currentsentinelThe desired behavior is what
TimeZonedoes today -.currentrepresents a snapshot in time and we should encode that fixed snapshot unless.autoupdatingCurrentis used (in which case it should update at decode time). This is trickier forLocalebecauseLocalecontains extra user preferences that are persisted neither in the identifier nor other archive keys (which is why it is not its behavior today). This means encodingLocale.currentas a fixed locale with just an identifier results in lost information due to lost preferences.To fix this we do two things:
Localenow persists preferences in the archive (when present) alongside the identifier (to prevent data loss) and.currentis no longer decoded as a sentinel to matchTimeZoneCalendarno longer decodes.currentas a sentinel value to matchTimeZone(and nowLocale)With this change, we always decode a
Calendarthat was encoded as.currentas a fixed Calendar based on the encoded values. We still encode the.currentkey to preserve the preexisting behavior when decoding on an older runtime, but newer runtimes will ignore the.currentsentinel and decode the various keys.This also adds unit tests to each type that validates consistent behavior for all 3 types.