On Tue, 26 Sep 2023 14:19:55 GMT, Technici4n <d...@openjdk.org> 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 1:
> 1: /*
You should update the ending copyright year of this file from 2022 to 2023.
src/java.base/share/classes/java/lang/module/Resolver.java line 621:
> 619:
> 620: String name = m1.name();
> 621: Set<ResolvedModule> m1Reads = entry.getValue();
Why not just this?
m1Reads.addAll(nameToResolved.values());
m1Reads.remove(m1);
src/java.base/share/classes/java/lang/module/Resolver.java line 639:
> 637: parent.configurations()
> 638: .map(Configuration::modules)
> 639: .flatMap(Set::stream)
Can't this and below step be `.forEach(m1Reads::addAll)`?
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337332185
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337342558
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337326631
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337350597
