On Tue, 12 Apr 2022 21:57:33 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> This is a redo of a recent fix, which passed all tier1 tests but broke "make
>> docs".
>>
>> The reason for the crash was a combination of a number of factors, such that
>> fixing any one (or more) would make the problem go away, making it hard(er)
>> to determine the correct path forward.
>>
>> 1. In a doc comment in JDK code, the author had mistakenly written a number
>> of tags like `{@link byte}` instead of `{@code byte}` and `{@link byte[]}`
>> instead of `{@code byte[]}`
>>
>> 2. `DocTrees.getElement` returns the javac-internal symbol for primitive
>> types (instead of `null`) and the base type for arrays. So for both `{@link
>> byte}` and `{@link byte[]}` the `LinkFactory` was given a javac-internal
>> symbol (instead of null). Filed as JDK-8284315.
>>
>> 3. Thinking the value was a declared type, and so in a named or unnamed
>> package, the `LinkFactory` attempts to create a link based on the package of
>> the value. The package for the java-internal symbols for primitive type
>> is a special `rootPackage` which has no equivalent in the Language Model
>> world, and which has a `null` owner.
>>
>> 4. The new code in `JavacElements.getModuleOf` assumes that (as is generally
>> standard in javac) all symbols have a non-`null` owner, this giving an NPE
>> when called with `rootPackage`.
>>
>> So, there are various possible ways to fix this.
>>
>> 1. put a broad null check for `sym.owner` in `JavacElements.getModuleOf`
>> 2. put a specific check for `rootPackage` in `JavacElements.getMOduleOf`
>> 3. fix `DocTrees.getElement` to not return javac-internal symbols
>> 4. change javadoc to workaround the issue with `DocTrees.getElement`
>> 5. fix the javadoc comments to not use bad forms, like `{@link byte}` etc.
>>
>> 1 and 2 would both be atypical idioms in the code.
>>
>> 3 is desirable, but may cause additional work/updates/etc. It would be a
>> change to the behavior of a public API, and so compatibility concerns would
>> need to be addressed, although arguably the change in behavior is a bug fix,
>> not a spec change. (The current behavior is under-specified!)
>>
>> By itself, 5 would just be ignoring the underlying problem.
>>
>> That leaves 4, which is the solution adopted here. It localizes the
>> change/check to javadoc internal code. It is an interim measure until we can
>> fix 3. (JDK-8284315)
>>
>> * The javac part of the fix is essentially the same as before (a redundant
>> import has been removed, this time.) By itself the code was not wrong, when
>> used as intended.
>>
>> * `DocTrees.getElement` is called from a single place in javadoc, in
>> `CommentHelper` and so a check is made on the result such that `null` is
>> returned when the result would otherwise not be a declared type.
>>
>> * A new (javadoc) test is added to verify the new behavior for `{@link
>> byte}` etc. The prior behavior (even before the original fix) was to display
>> the type in monospace font. The new behavior is for javadoc to fall back on
>> the recently-new mechanism to display a "invalid tag" block.
>>
>> With these fixes, "make docs" works, and does not crash(!) Comparing the
>> result of "make docs" before and after this fix, the only significant
>> differences arise from the 5 bad `{@link ...}` tags in `MemorySegment.java`.
>> These should (separately) be fixed.
>
> Jonathan Gibbons has updated the pull request incrementally with one
> additional commit since the last revision:
>
> address review feedback
Marked as reviewed by prappo (Reviewer).
-------------
PR: https://git.openjdk.java.net/jdk/pull/8101
