On Tue, 26 Sep 2023 14:54:37 GMT, Chen Liang <[email protected]> wrote:
>> Fixes the issue (hopefully) by resolving automatic modules and automatic
>> module dependencies after propagation of non-automatic transitive
>> dependencies. The module tests run.
>>
>> I also added a few asserts to validate that the automatic modules don't read
>> themselves (previously this was the case with > 1 automatic module). Should
>> these asserts be added in more places or is this enough?
>>
>> Alan also mentioned a conflict with the spec, I'm not sure which spec is
>> being referred to. The documentation of `ResolvedModule#reads` states `A
>> possibly-empty unmodifiable set`, implying that the set can be empty.
>>
>> Finally, `Configuration#reads` states `// The sets stored in the graph are
>> already immutable sets` but that does not seem to be true. Should something
>> be done about this to limit allocation?
>>
>> Please let me know!
>> Cheers
>
> src/java.base/share/classes/java/lang/module/Resolver.java line 665:
>
>> 663: } else {
>> 664: // parent configuration, already resolved
>> 665: // TODO: does this allocate a copy of the set every
>> time?
>
> It doesn't. `List.copyOf` and `Set.copyOf` returns the passed immutable
> collection instance if input is already an immutable collection.
The input did not seem immutable however
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1338177073
