Review request on the optional modules support

Alan Bateman Alan.Bateman at oracle.com
Mon Aug 8 08:01:16 PDT 2011


Mandy Chung wrote:
> An update on the optional module support [1].
>
> Webrev:
>    http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/optional-modules.00/
>
> The note has been updated to include the open questions raised in
> the discussion.
>    http://openjdk.java.net/projects/jigsaw/doc/topics/optional.html
I went through the webrev.

It mostly looks okay to me but I suspect that developers might initially 
grapple with isModulePresent and requireModulePresent as instance 
methods (in time I guess the javadoc will be fleshed out and concepts 
such as context can be referenced).

I see you've changed a few of the SecurityManager methods to throw 
ModuleNotPresentException but I'm not sure that is really needed. The 
alternative is to just have the permission checks fail, as they do now, 
when the desktop module is not present.

The updates to java.lang.reflect.Module remind me to ask whether this is 
the right package for this class (java.lang, java.lang.reflect or 
java.lang.module). I realize there is history here (should Class and 
Package be in java.lang.reflect, etc.) but it does seem inconsistent now.

One of the questions posed is whether the compiler can use the 
RequireOptionalModule annotations but I would have thought that if there 
are any references to types in optional modules then it will require 
that they be present at compile time. Maybe this question is only for 
the case that reflection is used?

Another question is about handling different versions but that strikes 
me as a bit fragile and could potentially get out of sync with the 
version constraints in the module-info too.

Otherwise I think the changes are okay and I note that 
RequireOptionalModule will need to have to classes element removed once 
we don't require the class analyzer in the build.

-Alan



More information about the jigsaw-dev mailing list