Skip to content

Commit 0127031

Browse files
committed
Address PR swiftlang#2028 review feedback
1 parent 52194a4 commit 0127031

6 files changed

Lines changed: 118 additions & 104 deletions

File tree

Sources/FoundationEssentials/Calendar/Calendar.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1304,10 +1304,15 @@ public struct Calendar : Hashable, Equatable, Sendable {
13041304
_calendar.nextDate(after: date, matching: components, direction: direction)
13051305
}
13061306

1307+
/// Whether the calendar implementation supports the `nextDate` fast path.
1308+
internal var _supportsNextDateFastPath: Bool {
1309+
_calendar.supportsNextDateFastPath
1310+
}
1311+
13071312
public func enumerateDates(startingAfter start: Date, matching components: DateComponents, matchingPolicy: MatchingPolicy, repeatedTimePolicy: RepeatedTimePolicy = .first, direction: SearchDirection = .forward, using block: (_ result: Date?, _ exactMatch: Bool, _ stop: inout Bool) -> Void) {
13081313
// Fast-path: drive the loop with direct nextDate calls when default policies are in effect.
13091314
if matchingPolicy == .nextTime && repeatedTimePolicy == .first,
1310-
let _ = _calendar.nextDate(after: start, matching: components, direction: direction) {
1315+
_supportsNextDateFastPath {
13111316
var current = start
13121317
var stop = false
13131318
while !stop {

Sources/FoundationEssentials/Calendar/CalendarConstants.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
/// Time-unit constants shared across calendar implementations.
14-
internal enum _CalendarConstants {
15-
static let kSecondsInWeek = 604_800
16-
static let kSecondsInDay = 86400
17-
static let kSecondsInHour = 3600
18-
static let kSecondsInMinute = 60
13+
extension Calendar {
14+
/// Time unit constants shared across calendar implementations.
15+
static let _kSecondsInWeek = 604_800
16+
static let _kSecondsInDay = 86400
17+
static let _kSecondsInHour = 3600
18+
static let _kSecondsInMinute = 60
1919

20-
/// Sentinel used by unbounded-range loops in date arithmetic.
21-
static let inf_ti: TimeInterval = 4398046511104.0
20+
/// Sentinel used by unbounded range loops in date arithmetic.
21+
static let _inf_ti: TimeInterval = 4398046511104.0
2222
}

Sources/FoundationEssentials/Calendar/Calendar_Enumerate.swift

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,13 +331,7 @@ extension Calendar {
331331
let validates = matchingComponents._validate(for: calendar)
332332
finished = !validates
333333

334-
// Probe: commit to the fast loop if the calendar recognizes this pattern.
335-
if validates && matchingPolicy == .nextTime && repeatedTimePolicy == .first,
336-
calendar._calendarNextDate(after: start, matching: matchingComponents, direction: direction) != nil {
337-
self.usesFastPath = true
338-
} else {
339-
self.usesFastPath = false
340-
}
334+
self.usesFastPath = validates && matchingPolicy == .nextTime && repeatedTimePolicy == .first && calendar._supportsNextDateFastPath
341335
}
342336

343337
mutating func next() -> Element? {
@@ -543,7 +537,7 @@ extension Calendar {
543537
previouslyReturnedMatchDate: Date?) throws -> SearchStepResult {
544538

545539
// Fast-path: ask the calendar directly. Returns nil for unrecognized patterns.
546-
if matchingPolicy == .nextTime && repeatedTimePolicy == .first {
540+
if _supportsNextDateFastPath && matchingPolicy == .nextTime && repeatedTimePolicy == .first {
547541
if let fast = _calendarNextDate(after: searchingDate, matching: matchingComponents, direction: direction) {
548542
return SearchStepResult(result: (fast, true), newSearchDate: fast)
549543
}

Sources/FoundationEssentials/Calendar/Calendar_Hebrew.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ internal final class _CalendarHebrew: _CalendarProtocol, @unchecked Sendable {
9494

9595
// hash(into:) uses the `_CalendarProtocol` default impl.
9696

97+
var supportsNextDateFastPath: Bool { true }
98+
9799
// MARK: - Range
98100

99101
// Year bounds: match ICU's Hebrew reporting of ±5M (covers full Int32 year range
@@ -368,10 +370,10 @@ internal final class _CalendarHebrew: _CalendarProtocol, @unchecked Sendable {
368370
case .era:
369371
// Hebrew has a single AM era spanning from epoch to effectively forever.
370372
// Matches ICU's reported start (Hebrew epoch, -181,778,083,200 s before
371-
// Date reference = year 1 AM, Tishrei 1, midnight UTC) and duration (_CalendarConstants.inf_ti).
373+
// Date reference = year 1 AM, Tishrei 1, midnight UTC) and duration (Calendar._inf_ti).
372374
return DateInterval(
373375
start: Date(timeIntervalSinceReferenceDate: -181_778_083_200.0),
374-
duration: _CalendarConstants.inf_ti
376+
duration: Calendar._inf_ti
375377
)
376378
case .year:
377379
// Tishri 1 of this year → Tishri 1 of next year.
@@ -514,7 +516,7 @@ internal final class _CalendarHebrew: _CalendarProtocol, @unchecked Sendable {
514516
let comps = dateComponents([.weekday, .hour, .minute, .second], from: date, in: self.timeZone)
515517
guard let dayOfWeek = comps.weekday else { return false }
516518
let timeInDay = TimeInterval(
517-
(comps.hour ?? 0) * _CalendarConstants.kSecondsInHour
519+
(comps.hour ?? 0) * Calendar._kSecondsInHour
518520
+ (comps.minute ?? 0) * 60
519521
+ (comps.second ?? 0)
520522
)

Sources/FoundationEssentials/Calendar/Calendar_Protocol.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,24 @@ package protocol _CalendarProtocol: AnyObject, Sendable, CustomDebugStringConver
5050
func date(byAdding components: DateComponents, to date: Date, wrappingComponents: Bool) -> Date?
5151
func dateComponents(_ components: Calendar.ComponentSet, from start: Date, to end: Date) -> DateComponents
5252

53-
/// Optional fast path for `Calendar.nextDate(after:matching:)`.
54-
/// Returns nil to fall through to the generic enumerate framework.
53+
/// Optional fast path for `Calendar.nextDate(after:matching:)`. Returns nil to fall through to the generic enumerate framework.
5554
func nextDate(after date: Date, matching components: DateComponents, direction: Calendar.SearchDirection) -> Date?
5655

56+
/// Whether this calendar implements `nextDate(after:matching:direction:)`.
57+
var supportsNextDateFastPath: Bool { get }
58+
5759
#if FOUNDATION_FRAMEWORK
5860
func bridgeToNSCalendar() -> NSCalendar
5961
#endif
6062
}
6163

6264
extension _CalendarProtocol {
6365
package func nextDate(after date: Date, matching components: DateComponents, direction: Calendar.SearchDirection) -> Date? {
64-
nil // default — fall through to Calendar's enumerate-based implementation
66+
nil
6567
}
6668

69+
package var supportsNextDateFastPath: Bool { false }
70+
6771
package var preferredFirstWeekday: Int? { nil }
6872
package var preferredMinimumDaysInFirstweek: Int? { nil }
6973

@@ -78,13 +82,7 @@ extension _CalendarProtocol {
7882
locale?.identifier ?? ""
7983
}
8084

81-
/// Default `hash(into:)` for calendars whose identity is the standard
82-
/// `(identifier, timeZone, firstWeekday, minimumDaysInFirstWeek,
83-
/// localeIdentifier, preferredFirstWeekday, preferredMinimumDaysInFirstweek)`
84-
/// tuple. Calendars with non-standard hashing semantics
85-
/// (e.g. `_CalendarAutoupdating`'s "all-equal" sentinel, `_CalendarICU`'s
86-
/// locked-accessor variant, `_CalendarBridged`'s underlying-`NSCalendar`
87-
/// hash) override this.
85+
/// Default `hash(into:)` for the standard calendar identity tuple. Calendars with non-standard hashing (e.g. `_CalendarAutoupdating`, `_CalendarICU`, `_CalendarBridged`) override this.
8886
package func hash(into hasher: inout Hasher) {
8987
hasher.combine(identifier)
9088
hasher.combine(timeZone)

Sources/FoundationEssentials/Calendar/Calendar_Recurrence.swift

Lines changed: 89 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -673,10 +673,7 @@ extension Calendar {
673673
return false
674674
}
675675

676-
/// Expand `_DateComponentCombinations` into a flat array of single-valued
677-
/// `DateComponents`. Negative ordinals are translated to `{month, weekday,
678-
/// weekOfMonth}` using `anchor`'s month structure. Returns nil if the
679-
/// pattern can't be expanded (too many combinations, `.every()` weekday, etc).
676+
/// Expand `_DateComponentCombinations` into a flat array of single-valued `DateComponents`. Negative ordinals are translated to `{month, weekday, weekOfMonth}` using `anchor`'s month structure. Returns nil if the pattern can't be expanded.
680677
fileprivate func _expandedDateComponents(
681678
_ c: _DateComponentCombinations,
682679
anchor: Date? = nil,
@@ -789,53 +786,77 @@ extension Calendar {
789786
return true
790787
}
791788

792-
func make(monthIdx: Int, weekdayIdx: Int,
793-
daysOfMonthIdx: Int, daysOfYearIdx: Int, weeksOfYearIdx: Int,
794-
hoursIdx: Int, minutesIdx: Int, secondsIdx: Int) -> DateComponents? {
795-
var dc = DateComponents()
796-
if let ms = c.months {
797-
dc.month = ms[monthIdx].index
798-
dc.isLeapMonth = ms[monthIdx].isLeap
789+
// Build a base DateComponents with single valued axes, then vary only the multi valued ones.
790+
var base = DateComponents()
791+
var axes: [(WritableKeyPath<DateComponents, Int?>, [Int])] = []
792+
793+
if let ms = c.months {
794+
if ms.count == 1 {
795+
base.month = ms[0].index
796+
base.isLeapMonth = ms[0].isLeap
797+
} else {
798+
axes.append((\.month, ms.map(\.index)))
799799
}
800-
if let woy = c.weeksOfYear { dc.weekOfYear = woy[weeksOfYearIdx] }
801-
if let doy = c.daysOfYear { dc.dayOfYear = doy[daysOfYearIdx] }
802-
if let dom = c.daysOfMonth { dc.day = dom[daysOfMonthIdx] }
803-
if let wds = c.weekdays {
804-
guard translateWeekday(wds[weekdayIdx], into: &dc) else { return nil }
800+
}
801+
if let woy = c.weeksOfYear {
802+
if woy.count == 1 { base.weekOfYear = woy[0] }
803+
else { axes.append((\.weekOfYear, woy)) }
804+
}
805+
if let doy = c.daysOfYear {
806+
if doy.count == 1 { base.dayOfYear = doy[0] }
807+
else { axes.append((\.dayOfYear, doy)) }
808+
}
809+
if let dom = c.daysOfMonth {
810+
if dom.count == 1 { base.day = dom[0] }
811+
else { axes.append((\.day, dom)) }
812+
}
813+
if let hs = c.hours {
814+
if hs.count == 1 { base.hour = hs[0] }
815+
else { axes.append((\.hour, hs)) }
816+
}
817+
if let mins = c.minutes {
818+
if mins.count == 1 { base.minute = mins[0] }
819+
else { axes.append((\.minute, mins)) }
820+
}
821+
if let secs = c.seconds {
822+
if secs.count == 1 { base.second = secs[0] }
823+
else { axes.append((\.second, secs)) }
824+
}
825+
826+
// Handle weekdays: translate each entry into the base or build a seed list.
827+
var seeds: [DateComponents]
828+
if let wds = c.weekdays {
829+
if wds.count == 1 {
830+
guard translateWeekday(wds[0], into: &base) else { return nil }
831+
seeds = [base]
832+
} else {
833+
seeds = []
834+
seeds.reserveCapacity(wds.count)
835+
for wd in wds {
836+
var wdBase = base
837+
guard translateWeekday(wd, into: &wdBase) else { return nil }
838+
seeds.append(wdBase)
839+
}
805840
}
806-
if let hs = c.hours { dc.hour = hs[hoursIdx] }
807-
if let mins = c.minutes { dc.minute = mins[minutesIdx] }
808-
if let secs = c.seconds { dc.second = secs[secondsIdx] }
809-
return dc
841+
} else {
842+
seeds = [base]
810843
}
811844

812-
var result: [DateComponents] = []
813-
result.reserveCapacity(total)
814-
for mIdx in 0..<monthsCount {
815-
for wIdx in 0..<weekdaysCount {
816-
for domIdx in 0..<daysOfMonthCount {
817-
for doyIdx in 0..<daysOfYearCount {
818-
for woyIdx in 0..<weeksOfYearCount {
819-
for hIdx in 0..<hoursCount {
820-
for miIdx in 0..<minutesCount {
821-
for sIdx in 0..<secondsCount {
822-
guard let dc = make(
823-
monthIdx: mIdx, weekdayIdx: wIdx,
824-
daysOfMonthIdx: domIdx,
825-
daysOfYearIdx: doyIdx,
826-
weeksOfYearIdx: woyIdx,
827-
hoursIdx: hIdx, minutesIdx: miIdx,
828-
secondsIdx: sIdx) else { return nil }
829-
result.append(dc)
830-
}
831-
}
832-
}
833-
}
834-
}
845+
// Expand each multi valued axis by cloning and patching.
846+
for (keyPath, values) in axes {
847+
var expanded: [DateComponents] = []
848+
expanded.reserveCapacity(seeds.count * values.count)
849+
for seed in seeds {
850+
for v in values {
851+
var dc = seed
852+
dc[keyPath: keyPath] = v
853+
expanded.append(dc)
835854
}
836855
}
856+
seeds = expanded
837857
}
838-
return result
858+
859+
return seeds
839860
}
840861

841862
/// Single-valued `DateComponents` from combinations, or nil if expansion is needed.
@@ -1040,41 +1061,35 @@ extension Calendar {
10401061
matchingPolicy: MatchingPolicy,
10411062
repeatedTimePolicy: RepeatedTimePolicy) throws -> [(Date, DateComponents)]? {
10421063

1043-
// Fast-path short-circuits. The protocol default for _calendarNextDate is nil,
1044-
// so non-Hebrew calendars fall through to the existing path unchanged.
1064+
// Fast-path short-circuits. Only fires when the calendar opts in.
1065+
if _supportsNextDateFastPath && matchingPolicy == .nextTime && repeatedTimePolicy == .first {
10451066

1046-
// (1) Single-combination: one value per field → single _calendarNextDate call.
1047-
if matchingPolicy == .nextTime && repeatedTimePolicy == .first,
1048-
let dc = _singleCombinationDateComponents(combinationComponents),
1049-
let fast = _calendarNextDate(after: startDate, matching: dc, direction: .forward) {
1050-
return [(fast, dc)]
1051-
}
1067+
// (1) Single-combination: one value per field.
1068+
if let dc = _singleCombinationDateComponents(combinationComponents),
1069+
let fast = _calendarNextDate(after: startDate, matching: dc, direction: .forward) {
1070+
return [(fast, dc)]
1071+
}
10521072

1053-
// (2) Multi-combination (positive ordinals): cartesian product → probe each.
1054-
if matchingPolicy == .nextTime && repeatedTimePolicy == .first,
1055-
let allDCs = _expandedDateComponents(combinationComponents) {
1056-
var results: [(Date, DateComponents)] = []
1057-
results.reserveCapacity(allDCs.count)
1058-
var allFastPathed = true
1059-
for dc in allDCs {
1060-
guard let fast = _calendarNextDate(after: startDate, matching: dc, direction: .forward) else {
1061-
allFastPathed = false
1062-
break
1073+
// (2) Multi-combination (positive ordinals): cartesian product.
1074+
if let allDCs = _expandedDateComponents(combinationComponents) {
1075+
var results: [(Date, DateComponents)] = []
1076+
results.reserveCapacity(allDCs.count)
1077+
var allFastPathed = true
1078+
for dc in allDCs {
1079+
guard let fast = _calendarNextDate(after: startDate, matching: dc, direction: .forward) else {
1080+
allFastPathed = false
1081+
break
1082+
}
1083+
results.append((fast, dc))
1084+
}
1085+
if allFastPathed && !results.isEmpty {
1086+
results.sort { $0.0 < $1.0 }
1087+
return results
10631088
}
1064-
results.append((fast, dc))
1065-
}
1066-
if allFastPathed && !results.isEmpty {
1067-
results.sort { $0.0 < $1.0 }
1068-
return results
10691089
}
1070-
}
10711090

1072-
// (3) Multi-combination with negative-ordinal translation.
1073-
if matchingPolicy == .nextTime && repeatedTimePolicy == .first,
1074-
_unadjustedDatesHasNegativeOrdinal(combinationComponents) {
1075-
var sentinel = DateComponents()
1076-
sentinel.weekday = 1
1077-
if _calendarNextDate(after: startDate, matching: sentinel, direction: .forward) != nil,
1091+
// (3) Multi-combination with negative ordinal translation.
1092+
if _unadjustedDatesHasNegativeOrdinal(combinationComponents),
10781093
let allDCs = _expandedDateComponents(combinationComponents, anchor: startDate) {
10791094
var results: [(Date, DateComponents)] = []
10801095
results.reserveCapacity(allDCs.count)

0 commit comments

Comments
 (0)