On Fri, 10 Mar 2023 19:30:45 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> The ModulePackages attribute is optional and an optimization to avoid
>> scanning the module contents to get the full set of packages. Tooling that
>> creates packaged modules (jar and jmod for now) will want the ModulePackages
>> attribute emitted always. So maybe the Classfile.buildModule methods should
>> be looked at again, at least I think the 2-arg and 3-arg methods should emit
>> the ModulePackages unconditionally, the 1-arg buildModule maybe not. This
>> isn't an issue for ModuleInfoWriter of course as it is only used by tests.
>
> `buildModules` is expected to be called with additional packages but instead
> it's called with all packages including all exported and open packages.
>
>
> /**
> * Build a module descriptor into a byte array.
> * @param moduleAttribute the {@code Module} attribute
> * @param packages additional module packages
> * @param handler a handler that receives a {@link ClassBuilder}
> * @return the classfile bytes
> */
> public static byte[] buildModule(ModuleAttribute moduleAttribute,
> List<PackageDesc> packages,
> Consumer<? super ClassBuilder> handler) {
>
>
> I checked the implementation that seems to match `@param packages` that
> expects additional module packages that are not exported nor open. If it
> intends to take additional packages, it will need to filter the exported and
> open packages at the callsite.
>
> Or the `packages` parameter lists all packages that will be used to create
> `ModulePackages` attribute. This seems to be easier to understand.
Maybe the variants of Classfile.buildModule need to be looked at again. For the
usage here, buildModule(ModuleAttribute, Consumer<? super ClassBuilder>) would
be more useful as it would allow all of the additional attributes to be emitted
in the handler rather than having buildModule making the decision on whether to
emit the ModulePackages attribute.
-------------
PR: https://git.openjdk.org/jdk/pull/11368
