Conversation
Signed-off-by: Craig Disselkoen <cdiss@amazon.com>
Signed-off-by: Craig Disselkoen <cdiss@amazon.com>
| @@ -269,12 +269,9 @@ public def durationSince (datetime other: Datetime) : Option Duration := | |||
| Int64.sub? datetime.val other.val | |||
|
|
|||
| public def toDate (datetime: Datetime) : Option Datetime := | |||
There was a problem hiding this comment.
Should we leave this implementations as it is? As a definition of the semantics of toDate I think the conditional logic is clearer than wondering what exactly Int64.smod does.
There was a problem hiding this comment.
I'm not sure I agree that the existing definition involving all of {div, *, mod} is easier to reason about than the new one. Maybe there's a third definition that's clearer than either
There was a problem hiding this comment.
I think it would also be nice to see the semantics remain the same while only the encoding is changed, I don't know how in general you view changes in the spec.
There was a problem hiding this comment.
I think the issue with smod is that you need to remember which kind of division it implements (truncated vs floored vs Euclidian), while this just keeps it explicit so I also find it more readable.
There was a problem hiding this comment.
I'll see how hard it is to prove the new encoding compliant with the old spec. See if I can get it done by EOD :)
4 participants
Refactors the definition of
Datetime.toDate(both in Spec and in SymCC) for efficiency.Some of this diff is just to resolve compiler warnings that currently exist on
main.