On Thu, 6 Apr 2023 20:24:27 GMT, Roger Riggs <[email protected]> wrote:
>> src/java.base/share/classes/jdk/internal/util/Architecture.java line 100:
>>
>>> 98: */
>>> 99: public static Architecture current() {
>>> 100: return archValues[OperatingSystemProps.CURRENT_ARCH_ORDINAL];
>>
>> I think the `Architecture ` is different from the `OperatingSystem`. There
>> are ports of [other
>> architectures](https://github.com/openjdk/jdk/blob/1d517afbd4547171ad6fb6a3356351c2554c8279/make/autoconf/platform.m4#L33-L188)
>> (Including `MIPS64el`, `LoongArch64`, `SPARC v9`, etc) in the upstream.
>>
>> Will hard coding architectures destroy these ports? Should a "OTHER" or
>> "UNKNOWN" entry be added to the enum to represent other architectures that
>> are not of concern?
>
> There is no benefit to preemptively defining a full set of architectures; we
> only need those that are used in the OpenJDK runtime selection of options or
> parameters.
> The other and unknown cases can be handled in code using switch as `default`
> or for `if (xx) {} else {...}` in the else clause.
> Ports not supported the OpenJDK can add enum values and the corresponding
> static `isXXX()` methods if needed.
>
> The template file used in the implementation could be renamed to be more
> agnostic to OS or arch.
I understand that there is no need to define enum entries for all architectures
now, but the `current` method seems to cause crashes on other platforms.
Even worse, `OperatingSystemProps` seems to be unable to compile on other
architectures at all:
https://github.com/openjdk/jdk/blob/52ca4a70fc3de9e285964f9545ea8cd54e2d9924/src/java.base/share/classes/jdk/internal/util/OperatingSystemProps.java.template#L40
Does `Architecture` really need to be implemented as an enum? The value of this
enum has never been used in this PR except for testing. I think perhaps just
providing the isXXX methods is enough.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1160242246
