On Mon, 31 Aug 2026 21:58:17 GMT, Naoto Sato <[email protected]> wrote:
>> This PR implements [JEP 540: Simple JSON API >> (Incubator)](https://openjdk.org/jeps/540). >> >> It adds the `jdk.incubator.json` module which provides APIs for reading and >> writing JSON documents as specified by [RFC >> 8259](https://datatracker.ietf.org/doc/html/rfc8259). This is an incubating >> API. >> >> API documentation: >> https://cr.openjdk.org/~naoto/json/javadoc/api/jdk.incubator.json/module-summary.html >> Co-authored-by: Justin Lu >> ([@justin-curtis-lu](https://github.com/justin-curtis-lu)), Stuart Marks >> ([@stuart-marks](https://github.com/stuart-marks)) >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Naoto Sato has updated the pull request incrementally with nine additional > commits since the last revision: > > - Merge remote-tracking branch 'jdk-sandbox/json' into > JDK-8381976-Implementation-for-Simple-JSON-API > - offset -> cursorPos in JsonParser > - Consolidated copyOf() in JsonArrayImpl constructor > - Make Utils.composeTypeError derive expected from Class name > - Refactor Frame classes in JG into records > - Give initial size to StringBuilder > - initSb() -> StringBuilder::new > - Making Util class final > - Clarify JSON string -> text in toDisplayString src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonNumberImpl.java line 46: > 44: private final LazyConstant<Optional<Integer>> numInteger = > LazyConstant.of(this::initNumInteger); > 45: private final LazyConstant<Optional<Long>> numLong = > LazyConstant.of(this::initNumLong); > 46: private final LazyConstant<Optional<Double>> numDouble = > LazyConstant.of(this::initNumDouble); Maybe use OptionalInt, OptionalLong, OptionalDouble for these? src/jdk.incubator.json/share/classes/jdk/incubator/json/impl/JsonValueSupport.java line 46: > 44: */ > 45: int offset(); > 46: } This is kind of a general comment, but I'll put it here. JsonValueSupport isn't much of an abstraction. It's a way to extract some commonality across the various JsonValue implementations when they have a "doc" and an "offset". But there's no abstraction here. The doc might be null if there's no underlying doc... except for JsonStringImpl, which has a "doc" but it might be from the JsonString.of() method instead of from parse(), so it's not really a doc, so for that case we have a separate boolean that makes sure not to return the doc if it came from a factory instead of from parse. What will put some pressure on this is if we eventually parse JSON from a byte array (which we likely presume to be encoded in UTF-8) and so the location will have a byte array and a byte offset. I think what's necessary at some point -- not immediately -- is a better abstraction for the document position of a JsonValue, and have it be an actual object. Then have a "null" object whose semantics are "no location" instead of passing and checking for (null, -1) in various places. And put the generation of the location information for an error message into that object instead of having disembodied logic in the Util class. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3900807106 PR Review Comment: https://git.openjdk.org/jdk/pull/32282#discussion_r3900804863
