On Tue, 18 May 2021 13:44:47 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> minor cleanup
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletManager.java
> line 264:
>
>> 262: "jdk.javadoc.internal.doclets.formats.html");
>> 263: pkgs.forEach(p -> thisModule.addOpens(p,
>> tagletLoaderUnnamedModule));
>> 264: }
>
> What's that and the corresponding WorkArounds.accessInternalAPI about?
It's Module System Magic. :-) Don't look too hard at the man behind the
curtains.
The general problem is allowing external code to access module internals if
they really want it and ask nicely. For external code that is provided on the
class path, there are command-line options like `--add-exports` and
`--add-opens`. These options allow a user to override access to
module-internal classes for classes in the application class loader, which
(colloquially) is the unnamed module for classes on the classpath.
The problem is that those command-line options do not grant access from
module-internal classes to classes in "other" unnamed modules ... yes, there
are many unnamed modules: essentially one per class loader. So, it is up to the
creator of any additional class loader to decide whether to grant privileges
to the classes in that class loader.
For javac, there are (regrettably) annotation processors and plugins that want
to access javac internals, and those classes are (you guessed) loaded into a
new class loader based on the `-processorpath` option. And so the decision was
made to provide an undocumented option to allow such code the same access that
they had before the module system was introduced. That javac option is
`-XDaccessInternalAPI`. Note that `-XD` is a semi-magic option that effectively
allows any `name=value` to be set in the internal options map, thus bypassing
the normal mechanism for documentated options. And, side-note, `javadoc`
provides the same `-XD` option for the same reasons. If you search the `javac`
code for the string `"accessInternalAPI"` you will find two usages, one for
plugins and one for anno-processors, and both result in a low-level utility
method being called, `ModuleHelper.addExports`, which gives various access
rights to any classes in that class loader.
Side-story: In JDK 9, `ModuleHelper` was much more complicated in its
implementation, because it had to work entirely through core-reflection,
because of the "bootstrap compiler" issue. Nowadays, it's much simpler.
Back to this PR. You've probably managed to guess most of the story at this
point. There's functionality missing from taglets, but I want to write a test
taglet that gets at the missing information. So, I've copied the javac
backdoor mechanism, and opened up access to some of the internal javadoc
packages, provided that a suitable command-line option is given. Further out,
we should provide some of the API that is currently missing, so that the test
code no longer needs access to doclet internals.
As for `WorkArounds`, that class is the only one we allow in the standard
doclet to access "tool stuff" via backdoors. In this case, `WorkArounds` needs
to access the compiler options, to see if `XDaccessInternalAPI` has been set.
Note that both the javac and javadoc mechanism are in line with JPMS policy
that you have to ask for the enhanced access by using command-line options.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line
> 474:
>
>> 472: */
>> 473: private DiagnosticPosition getDiagnosticPosition(DocTreePath path) {
>> 474: ToolEnvironment toolEnv = getToolEnv();
>
> toolEnv is unused.
Thanks, I'll double check and delete if unused.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolEnvironment.java
> line 147:
>
>> 145: enter = JavadocEnter.instance(context);
>> 146: names = Names.instance(context);
>> 147: externalizableSym = syms.enterClass(syms.java_base,
>> names.fromString("java.io.Externalizable"));
>
> What was externalizableSym about and why have you deleted it?
TL;DR: the value was unused. It's gratuitous cleanup ;-)
The value was the `Symbol` (javac subtype of `Element`) for the class
`java.io.Externalizable`. I presume at some point it was required for handling
serializable/externalizable classes, by checking if it was a supertype of a
class being documented.
> test/langtools/jdk/javadoc/doclet/testJavaFX/TestJavaFX.java line 422:
>
>> 420: "--javafx",
>> 421: "--disable-javafx-strict-checks",
>> 422: "--no-platform-links",
>
> It just happens so that yesterday I
> [asked](https://github.com/openjdk/jdk/pull/4051#pullrequestreview-660862882)
> Hannes about proliferation of `--no-platform-links` in tests and now we have
> 3 more of those in this PR alone.
I was getting more messages reporting that the classes could not be found. It
was throwing off warning counts in golden output. My unproven suspicion is that
we may not have been accurately counting warnings before this change.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4074
