On Fri, 10 Mar 2023 17:59:20 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> It'd be helpful to add a comment that `ModulePackages` attribute is only 
>> emitted if there are packages that aren't exported or open.
>
> 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.

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

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

Reply via email to