Thanks for the comments Maurizio.
On 28.11.2016 12:28, Maurizio Cimadamore wrote:
Hi,
the langtools code looks generally ok. Few questions:
* Why doesn't 'open' get its own directive in Directive.java - instead
of relying on a 'mode' set on an export directive?
It seemed to me that having two directive interfaces in the API for
directives that have the same structure was unnecessary (as we don't
have MethodElement and ConstructorElement, but just ExecutableElement,
or TypeElement that represents a class, an interface, an annotation type
or an enum type).
If you think it would be better to have separate interfaces for exports
and opens, I am OK with that as well.
* ClassReader: should we have checks regarding an open module containing
no open directives in the classfile? This seems to be called out in the
spec [1] - see section 2.2
I think such checks would be fine, working on a patch.
* At some point we should investigate better sharing strategy between
ClassReader and ModuleNameReader
* Names.dynamic seems unused
Fixed:
http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/8156e205fcb4
Jan
* I note that the classfile attribute name changes are not captured in
the spec (but I might be referring to a slightly older version).
Maurizio
[1] - http://cr.openjdk.java.net/~mr/jigsaw/spec/lang-vm.html#jigsaw-2.2
On 24/11/16 15:25, Alan Bateman wrote:
Folks on jigsaw-dev will know that we are on a mission to bring the
changes accumulated in the jake forest to jdk9/dev. We can think of
this as a refresh of the module system in JDK 9, the last big refresh
was in May with many small updates since then.
The focus this time is to bring the changes that are tied to JSR
issues into jdk9/dev, specifically the issues that are tracked on the
JSR issues list [1] as:
#CompileTimeDependences
#AddExportsInManifest
#ClassFileModuleName
#ClassFileAccPublic
#ServiceLoaderEnhancements
#ResourceEncapsulation/#ClassFilesAsResources
#ReflectiveAccessToNonExportedTypes
#AwkwardStrongEncapsulation
#ReadabilityAddedByLayerCreator
#IndirectQualifiedReflectiveAccess (partial)
#VersionsInModuleNames
#NonHierarchicalLayers
#ModuleAnnotations/#ModuleDeprecation
#ReflectiveAccessByInstrumentationAgents
Some of these issues are not "Resolved" yet, meaning there is still
ongoing discussion on the EG mailing list. That is okay, there is
nothing final here. If there are changes to these proposals then the
implementation changes will follow. Also, as I said in a mail to
jigsaw-dev yesterday [2], is that we will keep the jake forest open
for ongoing prototyping and iteration, also ongoing implementation
improvements where iteration or bake time is important.
For the code review then the focus is therefore on sanity checking the
changes that we would like to bring into jdk9/dev. We will not use
this review thread to debate alternative designs or other big
implementation changes that are more appropriate to bake in jake.
To get going, I've put the webrevs with a snapshot of the changes in
jake here:
http://cr.openjdk.java.net/~alanb/8169069/0/
The changes are currently sync'ed against jdk-9+146 and will be
rebased (and re-tested) against jdk9/dev prior to integration. There
are a number of small changes that need to be added to this in the
coming days, I will refresh the webrev every few days to take account
of these updates.
A few important points to mention, even if you aren't reviewing the
changes:
1. This refresh requires a new version of jtreg to run the tests. The
changes for this new version are in the code-tools/jtreg repository
and the plan is to tag a new build (jtreg4.2-b04) next week. Once the
tag has been added then we'll update the requiredVersion property in
each TEST.ROOT to force everyone to update.
2. For developers trying out modules with the main line JDK 9 builds
then be aware that `requires public` changes to `requires transitive`
and the `provides` clause changes to require all providers for a
specific service type to be in the same clause. Also be aware that the
binary form of the module declaration (module-info.class) changes so
you will need to recompile any modules.
3. Those running existing code on JDK 9 and ignoring modules will need
to be aware of a disruptive change in this refresh. The disruptive
change is #AwkwardStrongEncapsulation where setAccessible(true) is
changed so that it can't be used to break into non-public
fields/methods of JDK classes. This change is going to expose a lot of
hacks in existing code. We plan to send mail to jdk9-dev in advance of
this integration to create awareness of this change. As per the
original introduction of strong encapsulation then command line
options (and now the manifest of application JAR files) can be used to
keep existing code working. The new option is `--add-opens` to open a
package in a module for deep reflection by other modules. As an
example, if you find yourself with code that hacks into the private
`comparator` field in java.util.TreeMap then running with `--add-opens
java.base/java.util=ALL-UNNAMED` will keep that code working.
A few miscellaneous notes for those that are reviewing:
1. We have some temporary/transition code in the top-level repo to
deal with the importing of the JavaFX modules. This will be removed
once the changes are in JDK 9 for the OpenJFX project to use.
2. In the jdk repo then it's important to understand that the module
system is initialized at startup and there are many places where we
need to keep startup performance in mind. This sometimes means less
elegant code than might be used if startup wasn't such a big concern.
3. The changes in the jaxws repo make use of new APIs that means the
code doesn't compile with JDK 7 or JDK 8. Our intention is to work
with the JAXB and JAX-WS maintainers to address the issues in the
upstream project and then bring those changes into jdk9/dev to replace
the patches that we are forced to push for the short term.
4. You will see several tests where the value of the @modules tag has
`:open` or `:+open`. This is new jtreg speak. The former means the
test is run with --add-opens to open the package, the latter means the
test is exported at compile-time and exported + open at run-time (the
latter usage will be rare, it's where tests have static references to
JDK internal types and are also doing deep reflection with
setAccessible).
In terms of dates then we are aiming to integrate these changes into
jdk9/dev in early December. I will send a follow-up mail next week on
this as we work through the logistics.
-Alan
[1] http://openjdk.java.net/projects/jigsaw/spec/issues/
[2]
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2016-November/010219.html
