RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]
Jaikiran Pai
jpai at openjdk.org
Tue Apr 9 08:37:29 UTC 2024
On Tue, 9 Apr 2024 01:34:56 GMT, Naoto Sato <naoto at openjdk.org> wrote:
>> Hello Roger,
>>
>>> The code should reference the constants in Instant.java (though may need to make them package private.)
>>
>> The `ChronoField` class and the `Instant` class reside in separate packages, so making these two fields in `Instant`, package private will not help. I will have to make them public, which I think probably isn't a good idea. Unless you think we should do it? There is one other place already in the JDK where we have copy/pasted these values `src/java.base/share/classes/java/nio/file/attribute/FileTime.java`, so maybe we can continue with this copy/paste here too?
>>
>> As for the javadoc, after we decide about this field access detail, I'll update it accordingly to note that the values min and max range complies with the min and max epoch seconds supported by `Instant`.
>
> Should `INSTANT_SECONDS("InstantSeconds", SECONDS, FOREVER, ValueRange.of(Instant.MIN.getEpochSecond(), Instant.MAX.getEpochSecond())),
> ` work?
Hello Naoto, that's a very good point. I was too caught up with the constant values that it didn't occur to me that the `Instant.MIN` and `Instant.MAX` public fields could be used for this. I have followed your suggestion and updated the PR. I have also updated the javadoc to link to `Instant.MIN` and `Instant.MAX` as the supported epoch second range.
The test continues to pass with this change and fails (as expected) without the source change.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18674#discussion_r1557216492
More information about the core-libs-dev
mailing list