Open sffc opened 1 year ago
I think approach (3) should be a separate function. I think we should use preferably (2) but (1) is also fine.
I think date comparison is a complex situation anyway and we should have good docs telling people what the different options are. In such a scenario derive(Ord)
doesn't seem too bad.
But also, the RataDie type fixed the problem wrt bounds.
If people want to use these dates as BTreeMap keys then derive(Ord)
is probably better.
Proposals:
Consensus:
Date<AnyCalendar>
; we are not convinced that there are sufficiently motivated use cases for a general implementation, and clients like ECMAScript can implement their own.Date<AnyCalendar>
, including data comparison, including on whichever internal types are required to get it to bubble up to Date<AnyCalendar>
: AnyDateInner
and AnyCalendar
, including all DateInner types and all calendar types, with or without data. Make sure they are implemented optimally. We are assuming that we can make an efficient Eq by comparing data pointers; if not, we need to revisit the implementation with the group to decide the best path forward. OK to ignore the data in Hash. If a DateInner has a cached field, we should ignore the cache.PartialOrd
that returns None
if the calendar types are different.LGTM: @robertbastian @Manishearth @sffc @echeran @sotam1069 @atcupps
There are a few general approaches:
Temporal uses approach (3), but it's a non-starter for the
Ord
impl because it must be a total order.If we can get to (2), that's probably the best for ICU4X's purposes, but it's actually a bit tricky because there is not currently a space into which we can project the full range of dates in all calendars.
fixed_date
is limited toi32
, and the range of years in some calendars covers a different span than the range of years in others. One way to fix that problem is to changefixed_date
to be ani64
, which I think is a change we should make anyway.However, approach (1) works around both of these problems, and we can even use
derive(Ord)
. It's maybe not that bad from a user perspective, too.Another way to get out of this conundrum is to add different types of Ord functions like we did to Locale (#1709).
Discuss with:
Optional: