Hi Jon,
Thanks for the comments!
On 12.2.2019 02:27, Jonathan Gibbons wrote:
Why the call of modules.newRound() in JavadocTool line 202?
As I read it, nothing much should have happened to the modules up to
this point ... the code has just been parsing files, hasn't it?
ElementsTable.scanSpecifiedItems is calling initModules at the end, and
is invoked from:
etable.packages(packageNames)
.classTrees(classTrees.toList())
.scanSpecifiedItems();
I tried to remove that, but it turned out
ElementsTable.findModuleOfPackageName needs the module system to be
set-up, and is (eventually) called from ElementsTable.getFilesToParse.
So I've incline to let the module system set-up and then set it up
again, using newRound() to clear it between.
(If needed, we could keep the javadoc tool(s) untouched, by having one
more check in Modules.)
The use case looks interesting, because it does look like a
configuration error and so I was expecting more conflict detection, but
maybe that's a different separable problem. I guess if ever we had a
patch path for a module mX that pointed to a directory containing a
module-info.java containing mY, and we needed mX, we would get some sort
of "wrong module found" error. Here, we have the patch path matching the
sourcepath, but presumably we can't go round pairwise checking paths for
silly combinations.
I guess we could (and maybe should) have a task to introduce a warning
for cases like this. But javac knows from which module it was reading
the file from, so seemed more reliable to simply reuse that knowledge.
Jan
-- Jon
On 02/07/2019 08:42 AM, Jan Lahoda wrote:
Hi,
Consider code like this:
---src/module-info.java
module m { uses test.Test; }
---src/test/Test.java
package test; public class Test { }
And javac invocation like:
javac -sourcepath src --patch-module m2=src module-info.java
While analysing the module-info, javac will try to lazily/implicitly
load the test.Test class. And when Modules.setCompilationUnitModules
is called for test.Test, it will eventually call singleModuleOverride,
which will look at --patch-module to find if the given source is on
any module patch. And it will find out it is a module patch for m2,
but as javac never heard about m2 before, it will crash.
Seems that the biggest underlying problem is that even though javac
knows which modules owns the class, it will try to look for the owner
on the patch path. The proposed patch fixes that, and uses the correct
owning module. This requires also a little cleanup in javadoc.
Webrev: cr.openjdk.java.net/~jlahoda/8217868/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8217868
How does this look?
Thanks,
Jan
