Hi,

Based on some (offline) feedback and further testing, I've updated the langtools patch. The new webrev is here:
http://cr.openjdk.java.net/~jlahoda/8173777/langtools.02/

Delta webrev compared to .01:
http://cr.openjdk.java.net/~jlahoda/8173777/langtools.02/delta/webrev/index.html

The changes in this updated version:
-the previous version didn't allow the same file to be under both --patch-module and --module-source-path. The updated version allows this, and the --patch-module will prevail. This is currently only allowed when both --patch-module and --module-source-path agree on the name of the module, but this could be trivially removed if desired.

-for incremental compilation, the previous version required that the class output is also specified in --patch-module. The updated version will automatically use class output to read classfiles for patch modules (by virtue of ModuleSymbol.patchOutputLocation).

-the previous version didn't (always) report an error for module-info on --patch-module in multi-module mode. This should be fixed now, and there should be an error for module-info on patch location even in multi-module mode.

I was looking at explicit uses of -Xmodule: in other repositories, and there are some in the hotspot repository. All these appear to use a utility from the top-level repository:
test/lib/jdk/test/lib/InMemoryJavaCompiler.java
(there are other tests that use @compile/module in various repositories, including hotspot, but these need to be coordinated with jtreg). For now, I propose to tweak the utility to interpret -Xmodule: itself:
http://cr.openjdk.java.net/~jlahoda/8173777/top-level.00/

Eventually, we could stop using -Xmodule: inside this utility and pass the module name explicitly, e.g. encoded in the className, but for now this seems acceptable to me.

Feedback is welcome.

Thanks!

Jan


On 7.2.2017 19:11, Jan Lahoda wrote:
Hi,

Thanks.

I did two small updates to the patches:
-for langtools repository, I added unwrapping to
ClientCodeWrapper.WrappedJavaFileManager.getLocationForModule - the
incoming JavaFileObject is unwrapped, so that we don't pass our wrappers
to the client's code. Direct link:
http://cr.openjdk.java.net/~jlahoda/8173777/langtools.01/src/jdk.compiler/share/classes/com/sun/tools/javac/api/ClientCodeWrapper.java.udiff.html


-for jdk repository, I fixed the:
test/javax/xml/jaxp/common/8035437/run.sh
to use a platform specific path separator. This is based on code from
jdk/test/java/lang/instrument/appendToClassLoaderSearch/CommonSetup.sh
Direct link:
http://cr.openjdk.java.net/~jlahoda/8173777/jdk.02/test/javax/xml/jaxp/common/8035437/run.sh.udiff.html


(Sorry that I missed this before.)

Updated webrevs:
-langtools repository:
http://cr.openjdk.java.net/~jlahoda/8173777/langtools.01/
-jdk repository:
http://cr.openjdk.java.net/~jlahoda/8173777/jdk.02/

Thanks,
    Jan

On 6.2.2017 21:42, Alan Bateman wrote:


On 06/02/2017 18:41, Jan Lahoda wrote:
I've updated the jdk repository webrev to wrap long lines:
http://cr.openjdk.java.net/~jlahoda/8173777/jdk.01/

Jan
This looks okay to me.

-Alan

Reply via email to