RFR: 8320716: ResolvedModule::reads includes self when configuration contains two or more automatic modules [v2]

Alan Bateman alanb at openjdk.org
Thu Nov 30 19:58:27 UTC 2023


> This is update to the specification of the j.l.module.ResolvedModule.reads method to clarify that the set of resolved modules returned does not include itself. There is a small implementation change to align with the specification and fix an anomaly that arises with configurations that contain two or more automatic modules.
> 
> Every module reads itself but the intent with ResolvedModule.reads is that it returns a set that doesn't include self. As things stand right now, the returned set does not include self when all modules in the configuration are explicit modules.  However, if the configuration contains two or more automatic modules then the set includes self, a side effect of augmenting the readability graph due to implied readability.
> 
> The specification of the "reads" method is updated. The implementation is also changed to skip automatic modules when augmenting the graph due to implied readability. It is skipped as each automatic module already reads all other modules. Note that it is not goal here to change the algorithm for handling implied readability, this may be a topic for a follow on PR.
> 
> The existing ConfigurationTest already includes several tests for configurations consisting solely of explicit modules, no updates are needed. For configurations that include automatic modules, the existing AutomaticModulesTest is updated to add new asserts to each of the testConfigurationXXX methods. I decided to migrate this test to JUnit while visiting, most of it is just migrating the TestNG data providers to be parameterized tests. If needed then we could separate this but it's a simple migration so I left it in.

Alan Bateman 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 four additional commits since the last revision:

 - Add comment to say that automatic module already reads all selected modules
 - Merge
 - Reorder asserts
 - Initial commit

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/16818/files
  - new: https://git.openjdk.org/jdk/pull/16818/files/0330f005..53c4f068

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16818&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16818&range=00-01

  Stats: 19018 lines in 528 files changed: 13858 ins; 3199 del; 1961 mod
  Patch: https://git.openjdk.org/jdk/pull/16818.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16818/head:pull/16818

PR: https://git.openjdk.org/jdk/pull/16818


More information about the core-libs-dev mailing list