On Tue, 28 Apr 2026 14:21:21 GMT, Jorn Vernee <[email protected]> wrote:

>> Per Minborg has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 60 additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into 
>> implement-lazy-constants-third-prewiew-exceptions
>>  - Merge branch 'openjdk:master' into master
>>  - Improve TestSegmentOffset and convert to junit
>>  - Address comments in PR
>>  - Change the way throwing toString is handled
>>  - Improve LazyMap tests
>>  - Harden tests
>>  - Update code style in if branch
>>  - Reorder modifiers
>>  - Merge branch 'master' into 
>> implement-lazy-constants-third-prewiew-exceptions
>>  - ... and 50 more: https://git.openjdk.org/jdk/compare/bba732fe...f67028cd
>
> src/java.base/share/classes/java/lang/LazyConstant.java line 235:
> 
>> 233:      * section.
>> 234:      *
>> 235:      * @throws NoSuchElementException if this lazy constant is in an 
>> error state
> 
> I wonder if it's useful to add a corresponding `throws` clause to the method 
> declaration. I thought javadoc checked this as well, but maybe that's just 
> for checked exceptions.

Yes, I think it is only for checked exceptions. See `Map::get` as an example.

> src/java.base/share/classes/java/lang/LazyConstant.java line 300:
> 
>> 298:      *             : LazyConstant.of(computingFunction);
>> 299:      *     }
>> 300:      * }
> 
> This section doesn't feel needed to me. Whether the factory is idempotent or 
> not seems like a pretty insignificant implementation detail, and the javadoc 
> already states that a new instance is returned.

I've gotten a lot of feedback/suggestions to return `this` if the `Supplier` 
already is a `LazyConstant` so that's why I added this. To avoid such comments 
in the future and to clearly communicate this was a deliberate design decission.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r3161232904
PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r3161244158

Reply via email to