<div dir="ltr">Hi jon,<div><br></div><div>Thank you for your comment. I have not forgotten this thread. I want to see some more opinions and do more investigation.</div><div>I can't guarantee that I can solve the issue in the end. But the discussion about this issue is good for us.</div><div><br></div><div>Best Regards.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Dec 19, 2020 at 6:17 AM Jonathan Gibbons <<a href="mailto:jonathan.gibbons@oracle.com">jonathan.gibbons@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Hi,</p>
<p>Thanks for your interest in this area.<br>
</p>
<p>The bugs, such as they are, are more in the specification than
the implementation, and are somewhat tricky to fix as well as
might be expected. There is one task that is part bug, part
enhancement, depending on your point of view, and equally tricky
to figure out, which is one of the reasons it has not been done
before now.</p>
<p>Generally, you should not think of the `JavaFileManager` API and
`JavaCompiler.getTask` options as equivalent. The file manager is
a lower level abstraction than the compiler, and applicable in
more contexts than `javac`. By itself, it is not wrong for a file
manager to have combinations of locations set, because there may
be situations (i.e. some tools) where that is desirable and
appropriate. And, incorrect settings for a file manager are
generally reported by throwing `IllegalArgumentException` in
preference to using a diagnostic listener. And, while it does not
make sense to allow conflicting options like `--source-path` and
`--module-source-path` on the javac command line, it seemed better
to permit file managers with both set and to figure a reasonable
policy to handle that situation.<br>
</p>
<p>Also, note that when using the Compiler API and
`JavaCompiler.getTask`, the compiler has to be able to accept any
suitable implementation of a file manager, and not just the
standard one returned by the JDK implementation of
`ToolProvider.getSystemJavaCompiler().getStandardJavaFileManager`.
And, for better or worse, there is no API on a file manager to
determine if errors have occurred, in a way that would be
reasonable for the internals of javac to detect and react to.</p>
<p>Your comments related to your example with `task1` and `task2`
are somewhat reasonable. Although it is on one hand not surprising
that options to getTask affect the file manager, on the other, it
would be nice if the changes were transient. The general idea we
have had in the past is to create a temporary file manager inside
the compiler that wraps and delegates to the given file manager,
except for the arguments given to the options. However we have not
thought through any detailed design to make that happen.</p>
<p>As for the specification, yes, the API in this area is somewhat
under-specified. Part of it is a general problem with JDK API
specifications: both JavaCompiler and JavaFileManager are
service-provider interfaces, and while it is easy enough to
specify the common properties of all implementations of these
interfaces, there is not a good way/place to specify the
additional characteristics and properties of specific
implementations. For the JavaCompiler API, we want to allow other
vendors to provide alternate implementations of a Java Compiler
(subject to all the appropriate conformance rules), and so it is
inappropriate in the Java SE class `javax.tools.JavaCompiler` to
give JDK-specific details of the JDK implementation of that
interface. And, for any implementations of `JavaFileManager`, we
don't want to over-specify it and so restrict the kinds of
implementations anyone can provide. The one thing of note is that
whereas there was no good solution to this specification problem
in JDK 6, since JDK 9, and the Java Platform Module System, we do
have a reasonable place to put some of this specification, and
that is the module documentation for the `jdk.compiler` module. In
that module, we could write paragraphs that give additional
specification details for the implementation of the `JavaCompiler`
interface provided by the module, and for the standard file
manager returned by the `getStandardJavaFileManager` method of
that implementation. It's "not great" that the documentation with
be narrative paragraphs not directly linked into the shared
documentation in the `javax.tools` interface, but at least it
would be somewhere, which is better than now.</p>
<p>-- Jon<br>
</p>
<div>On 12/18/20 6:38 AM, Guoxiong Li wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>When I was working on the JDK-8232939[1], I found some
strange points and suspicious bugs. The origin of the issues
is the `Context` of `StandardJavaFileManager`(eg:
JavacFileManager) and the `Context` of `CompilationTask`(eg:
JavacTaskImpl) is not same. As we can see in class
`JavacTool`, the methods `getStandardFileManager` and
`getTask` both create its new `Context`.</div>
<div><br>
</div>
<div>When creating its new `Context` to isolate. We can call it
as the <b>decoupled</b>Â part of `StandardJavaFileManager` and
`CompilationTask`.</div>
<div>When using `getTask` with a file manager to get a
`CompilationTask`, the `getTask` will set some properties of
the file manager. We can call it as the <b>coupled</b> part
of `StandardJavaFileManager` and `CompilationTask`.<br>
</div>
<div><br>
</div>
<div>The decoupled and coupled part of `StandardJavaFileManager`
and `CompilationTask` are strange and both have suspicious
bugs which are shown below.</div>
<div><br>
</div>
<div>1. Bugs about the decoupled part.</div>
<div>  The properties of `JavacFileManager`, especially `Log`
and `Options`, are not same as the corresponding items of
`JavacTaskImpl`. Some issues will occur.</div>
<div><br>
</div>
<div>Â Â The first example is that the OptionGroup.FILEMANAGER
options won't be validated. This is a bug about the different
`Options`.</div>
<div>Â Â As you can see the test code[2]. `--module-source-path`
and `--source-path` are both set. It is wrong because the man
document states that users can use only one of
`--module-source-path` and `--source-path`, and can't use
both. These wrong code are accepted by the compiler now.
Because when using the compiler API, the options property of
file manager only contain the OptionGroup.FILEMANAGER options
and the options property of compiler task only contain
the OptionGroup.BASIC options. When validating options[3],
only the options of compiler task will be validated. That is
said, the OptionGroup.FILEMANAGER options won't be validated.
It is unacceptable because the manner of compiler API is
different from the compiler command line.</div>
<div><br>
</div>
<div>  The second example is JDK-8232939[1]. It is a bug about
the different `Log.`</div>
<div>Â Â The file manager generates the errors during the parse
stage and reports the error by using its own `Log`. After the
parse stage, the compiler uses `stopIfError` to check if the
errors occur and will stop the later stages if any error
occurs. Unfortunately, the method `stopIfError` checks the
`Log` of the compiler instead of the `Log` of the file manager
so that it can't find any error generated by the file manager.
So the compiler continues the later stages unexpectedly.</div>
<div><br>
</div>
<div>Â Â These two bugs(one about `Options`, another about
`Log`) show that current decoupled way is not so good. If we
deep into all the properties of these two different context,
some more bugs may be found.</div>
<div><br>
</div>
<div>2. Bugs about the coupled way.</div>
<div>Â Â This aspect are more important and more unacceptable.
As you can see the code snippet below. </div>
<div>Â Â Â Â ```</div>
<div>Â Â Â Â var compiler =
ToolProvider.getSystemJavaCompiler();<br>
    var fm = compiler.getStandardFileManager(null, null,
null);<br>
    var files =
fm.getJavaFileObjects(Path.of("Test.java"));<br>
    JavaCompiler.CompilationTask task1 =
compiler.getTask(null, fm, null, List.of("-XDrawDiagnostics",
"-encoding", "ASCII"), null, files);<br>
    JavaCompiler.CompilationTask task2 =
compiler.getTask(null, fm, null,
List.of("-XDrawDiagnostics"/*some more options*/), null,
files);<br>
    System.out.println(task1.call());<br>
    System.out.println(task2.call());<br>
</div>
<div>Â Â Â Â ```</div>
<div>   The OptionGroup.FILEMANAGER options of
`task1`("-encoding", "ASCII") will be taken to `task2` andÂ
vice versa. Because the options of file manager won't be
cleared. It required the users clear the options manually.
Unfortunately, the users, included me, use it incorrectly. The
probably correct usage is shown below: run the first task and
clear the information of previous task before getting another
task. As you can see, the clear work is hard because the users
don't know the internal of the compiler. Currently I can't
find a good way to clear, too. </div>
<div>Â Â Â Â ```</div>
<div>Â Â Â Â var compiler =
ToolProvider.getSystemJavaCompiler();<br>
    var fm = compiler.getStandardFileManager(null, null,
null);<br>
    var files =
fm.getJavaFileObjects(Path.of("Test.java"));<br>
    JavaCompiler.CompilationTask task1 =
compiler.getTask(null, fm, null, List.of("-XDrawDiagnostics",
"-encoding", "ASCII"), null, files);<br>
    System.out.println(task1.call());</div>
<div>Â Â Â Â // Do some clear work. Omit.<br>
    JavaCompiler.CompilationTask task2 =
compiler.getTask(null, fm, null, List.of("-XDrawDiagnostics"),
null, files);<br>
    System.out.println(task2.call());</div>
<div>Â Â Â Â ```</div>
<div>Â Â As the document stated:</div>
<div>Â Â >Â The standard file manager serves two purposes:</div>
<div>Â Â >Â Â Â basic building block for customizing how a
compiler reads and writes files</div>
<div>Â Â >Â Â Â sharing between multiple compilation tasks</div>
<div>  But the options and task's own individual other
information perhaps shouldn't be shared.</div>
<div><br>
</div>
<div><br>
</div>
<div>To solve the problem completely, we should find a better
way to decouple file manager and compilation task instead of
using the new `Context` directly and simply.</div>
<div>I have been thinking of a better solution these days but I
can't find a satisfying solution. I want to share the problem
with you to see your opinions. This is the reason of this
email.</div>
<div><br>
</div>
<div>Any opinion or idea are appreciated!<br>
</div>
<div><br>
</div>
<div>[1]Â <a href="https://bugs.openjdk.java.net/browse/JDK-8232939" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8232939</a></div>
<div>[2]Â <a href="https://github.com/openjdk/jdk/blob/853c04712d24788b6d30d54443ac9277a898311b/test/langtools/tools/javac/modules/AnnotationProcessing.java#L1607" target="_blank">https://github.com/openjdk/jdk/blob/853c04712d24788b6d30d54443ac9277a898311b/test/langtools/tools/javac/modules/AnnotationProcessing.java#L1607</a></div>
<div>[3]Â <a href="https://github.com/openjdk/jdk/blob/853c04712d24788b6d30d54443ac9277a898311b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#L409" target="_blank">https://github.com/openjdk/jdk/blob/853c04712d24788b6d30d54443ac9277a898311b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java#L409</a></div>
<div><br>
</div>
</div>
</blockquote>
</div>
</blockquote></div>