search

haskell / time

A time library
http://hackage.haskell.org/package/time
Other
118 stars 78 forks source link

TH Lift instances for time types #233

Closed kokobd closed 1 year ago

kokobd commented 1 year ago

Can we provide instances of Language.Haskell.TH.Syntax.Lift for types in time, like UTCTime, DiffTime, etc, so that time can be used in TH splices. If there aren't major blockers (should time depend on template-haskell?), I'll be happy to submit a PR.

AshleyYakeley commented 1 year ago

Probably better to put them in the template-haskell package? I'm not sure, though.

JackKelly-Bellroy commented 1 year ago

My gut instinct is that template-haskell is a domain-independent library, and so it seems weirder to for users of template-haskell to depend on time than vice-versa. template-haskell seems pretty locked to specific GHC releases, whereas adding new instances to time should be a bit easier to publish? (Sadly, th-compat will be unusable here due to dep footprint, so we'll probably have to CPP it like time-qq does.)

AshleyYakeley commented 1 year ago

Can we use DeriveLift?

JackKelly-Bellroy commented 1 year ago

https://github.com/brendanhay/amazonka/pull/677/files

Looks like it, for everything except DiffTime, which does not export its constructor.

AshleyYakeley commented 1 year ago

OK, checking with the CLC in case there's a problem with time depending on template-haskell.

AshleyYakeley commented 1 year ago

I think the dependency is OK. You can make a PR, however, there may not be instance Lift Fixed, which should be added to template-haskell.

kokobd commented 1 year ago

I've raised an issue in template-haskell: https://gitlab.haskell.org/ghc/ghc/-/issues/22953. I think we can first release a version without relying on instance Lift Fixed. Instead, manually implement Lift for DiffTime using diffTimeToPicoseconds. This also makes time compatible with older versions of template-haskell.

JackKelly-Bellroy commented 1 year ago

The primary problem with lifting DiffTime is that it doesn't export its contstructor. The way Amazonka works around it is to call fromEnum, lift that, then call toEnum. Seems to work fine.