Skip to content
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

Names of Never types in datetime #5689

Open
sffc opened this issue Oct 15, 2024 · 2 comments · Fixed by #5693
Open

Names of Never types in datetime #5689

sffc opened this issue Oct 15, 2024 · 2 comments · Fixed by #5693
Labels
C-datetime Component: datetime, calendars, time zones

Comments

@sffc
Copy link
Member

sffc commented Oct 15, 2024

Never types are types used as a placeholder when the thing they represent is not needed.

They are used in the following cases in neo datetime:

  • The absence of a calendar (NeverCalendar)
  • When a field is not needed or returned for a field set (NeverField)
  • When a data payload is not to be loaded (NeverMarker)

Some, but not all, of the time, these can be the unit type (). However, when they need to implement a trait, it is sometimes required that they be their own type, since there are restrictions to implementing traits on standard library types (unless we restrict ourselves to using only our own traits).

I try my best to hide these so that they don't show up in user code, but they are part of public API and they could end up there in some cases.

Help me decide:

  1. Should there be one Never type per crate or one per use case?
  2. Should I prefer () always (even if it means a less natural trait impl), Never always (even if () could have been used), or decide on a case-by-case basis?
  3. Should they be named Phantom instead of Never? We use the word "never" in some enums to mean "never show the sign", for example, and "NeverCalendar" just looks weird; "PhantomCalendar" might be more intuitive.

This probably blocks 2.0 beta because I need to name these things.

@robertbastian @Manishearth

@sffc sffc added the discuss-priority Discuss at the next ICU4X meeting label Oct 15, 2024
@Manishearth
Copy link
Member

I think Never as "a type level Option::None" is a good association, and having NeverMarker as a global thing works nicely. I think it's good to use () where possible, even if it's a little clunky (but we can discuss if it feels super clunky).

I think Phantom means something different from Never, Phantom has a much stronger association towards low level abstractions and variance for me in Rust.

@sffc sffc mentioned this issue Oct 16, 2024
@sffc sffc closed this as completed in c7194ee Oct 17, 2024
@sffc sffc added C-datetime Component: datetime, calendars, time zones and removed discuss-priority Discuss at the next ICU4X meeting labels Oct 18, 2024
@sffc sffc added this to the ICU4X 2.0 ⟨P1⟩ milestone Oct 18, 2024
@sffc
Copy link
Member Author

sffc commented Oct 18, 2024

There is still NeoNeverMarker.

@sffc sffc reopened this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-datetime Component: datetime, calendars, time zones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants