On Mon, 13 Mar 2023 19:18:26 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>>> I see, I'll move the "packages magic" back from Classfile API to the >>> `ModuleInfoWriter` and correct the API accordingly. >> >> I would need to check the history in the jake repo but I think it dates from >> a prototype version of that attribute that only listed the concealed >> packages. I think it should be okay to just drop this from ModuleInfoWriter >> now. As I mentioned in another comment, the only user now is tests so you >> should able to just remove this code so that it doesn't generate the >> ModulePackages attributes. > >> I think it would be better to change is so that calling it with >> ModuleAttribute will emit a Module attribute. The overloads that take a >> Module plus a set of packages can emit a Module + ModulePackage >> unconditionally. > > I'm fine with this. > > `buildModule(ModuleAttribute moduleAttribute, List<PackageDesc> packages, > Consumer handler)` would emit `ModulePackages` unconditionally. It means > that it would need another `buildModule` variant that takes ModuleAttribute > and handler only if no `ModulePackages` is generated. `buildModule` argument `List<PackageDesc> packages` was explicitly added only for the purpose of this conditional packages calculation. If the `ModulePackages` attribute is optional and its content is not calculated, there is no more reason for this argument. I would propose to reduce the method to `buildModule(ModuleAttribute moduleAttribute, Consumer handler)`. `ModulePackages` should be then built using regular approach inside the handler: `classBuilder.with(ModulePackagesAttribute.of(...));`. ------------- PR: https://git.openjdk.org/jdk/pull/11368
