MLton / mlton

The MLton repository
http://mlton.org
Other
960 stars 127 forks source link

Date.localOffset on eastern regions #499

Closed minoki closed 1 year ago

minoki commented 1 year ago

I expected Date.localOffset () to return negative time for eastern regions (e.g. ~32400.000 for UTC+9), but MLton returns a positive value.

Code:

print ("localOffset=" ^ Time.toString (Date.localOffset ()) ^ "\n");

Expected output on UTC+9:

localOffset=~32400.000

Actual output on UTC+9:

localOffset=54000.000
MatthewFluet commented 1 year ago

The SML-side of MLton's implementation of Date.localOffset is just a thin wrapper around the Data_localOffset C function defined in ./runtime/basis/System/Date.c:

https://github.com/MLton/mlton/blob/f0c004970986a2ac68dd4ac9426eefc75f3748a1/runtime/basis/System/Date.c#L32-L39

It does seem as though eastern regions are expected to be negative, based on the comments in ./basis-library/system/date.sml:

https://github.com/MLton/mlton/blob/f0c004970986a2ac68dd4ac9426eefc75f3748a1/basis-library/system/date.sml#L104-L105

Most of these "system" functions are meant to be thin wrappers around C Library functions; is there such a function that we should be using, rather than the implementation above?

minoki commented 1 year ago

The "thin wrapper" part may be buggy:

https://github.com/MLton/mlton/blob/f0c004970986a2ac68dd4ac9426eefc75f3748a1/basis-library/system/date.sml#L303

Shouldn't it be rem instead of mod?

MatthewFluet commented 1 year ago

That might work. I need to look at the specifications of the various functions involved more carefully.