RFR: JDK-8283714: REDO - Unexpected TypeElement in ANALYZE TaskEvent [v2]

[Pavel Rappo]

On Tue, 12 Apr 2022 21:57:33 GMT, Jonathan Gibbons <jjg at 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