Skip to content

Commit

Permalink
Update isEarlierThan to consider NEVER
Browse files Browse the repository at this point in the history
  • Loading branch information
erlingrj committed Nov 22, 2023
1 parent 7689b93 commit 11d1dd9
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core/src/main/java/org/lflang/TimeValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ private static long makeNanosecs(long time, TimeUnit unit) {

/** Returns whether this time value is earlier than another. */
public boolean isEarlierThan(TimeValue other) {
if (this == NEVER && other != NEVER) {
return true;
} else if (this != NEVER && other == NEVER) {
return false;
}
return this.compareTo(other) < 0;
}

Expand Down

4 comments on commit 11d1dd9

@edwardalee
Copy link
Collaborator

@edwardalee edwardalee commented on 11d1dd9 Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the C runtime, NEVER is encoded as the largest negative number. In the Java code, however, we have this:

  public static final long MAX_LONG_DEADLINE = Long.decode("0x7FFFFFFFFFFF");
...
  public static final TimeValue NEVER = new TimeValue(Long.MAX_VALUE, TimeUnit.NANO);
  public static final TimeValue FOREVER = new TimeValue(MAX_LONG_DEADLINE, TimeUnit.NANO);

These look like odd definitions to me. The definition of NEVER is effectively limiting the TimeValue to 48 bits, in which case we will get overflow far sooner than the 292 years.
Anybody remember why these choices? Wouldn't it be better to encode NEVER as Long.MIN_VALUE?

@erlingrj
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I was trying to mimic the C runtime with NEVER = 0xFFFFF..... and FOREVER=0x7FFFFFF....

@erlingrj
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it here, thanks for spotting this. I thought Long was unsigned....

@erlingrj
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.