Bug 558004 - Several NPEs when compiling module declarations after 549855
Summary: Several NPEs when compiling module declarations after 549855
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.14   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: 4.15 M1   Edit
Assignee: Christoph Langer CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2019-12-08 17:33 EST by Christoph Langer CLA
Modified: 2019-12-16 03:27 EST (History)
7 users (show)

See Also:
manoj.palat: review+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Langer CLA 2019-12-08 17:33:42 EST
As a regression of https://bugs.eclipse.org/bugs/show_bug.cgi?id=549855 I observe several NPE issues when parsing module-info files of OpenJDK, e.g.

http://hg.openjdk.java.net/jdk/jdk/file/fb39a8d1d101/src/jdk.zipfs/share/classes/module-info.java
http://hg.openjdk.java.net/jdk/jdk/file/fb39a8d1d101/src/java.base/share/classes/module-info.java

For instance there is:

java.lang.NullPointerException
	at org.eclipse.jdt.internal.compiler.ast.Javadoc.resolveReference(Javadoc.java:426)
	at org.eclipse.jdt.internal.compiler.ast.Javadoc.resolve(Javadoc.java:293)
	at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.resolve(CompilationUnitDeclaration.java:645)
	at org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:901)
	at org.eclipse.jdt.internal.compiler.ProcessTaskManager.run(ProcessTaskManager.java:145)
	at java.base/java.lang.Thread.run(Thread.java:830)


The general problem is that a module declaration's scope is a MethodScope without an Enclosing ClassScope. So far, the assumption in org.eclipse.jdt.internal.compiler.ast.Javadoc in several places is that there is some parent ClassScope somewhere and null is not an option.

I'd like to propose a fix which checks for null at some more places. It would only be a short-term band-aid. We should generally consider whether a module declaration's scope should be a MethodScope and not something else (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=553472)

Maybe also in the context of https://bugs.eclipse.org/bugs/show_bug.cgi?id=548069.

I'd hope that my proposed fix can make it into RC2 - otherwise it would be a nasty regression for the OpenJDK usecase.
Comment 1 Eclipse Genie CLA 2019-12-08 17:38:34 EST
New Gerrit change created: https://git.eclipse.org/r/154079
Comment 2 Stephan Herrmann CLA 2019-12-08 17:54:32 EST
Unfortunately RC2 has already sailed, see bug 553793.

Only in cases of severe damage to life or property we could request a respin.

As a workaround you may have to disable javadoc analysis in the preferences.


Did you see that Vikas created bug 553564 to follow up from bug 553472 in what I called the mid-term solution?
Comment 3 Christoph Langer CLA 2019-12-08 18:13:48 EST
(In reply to Stephan Herrmann from comment #2)
> Unfortunately RC2 has already sailed, see bug 553793.
> 
> Only in cases of severe damage to life or property we could request a respin.
>
Oh, too bad.
 
> As a workaround you may have to disable javadoc analysis in the preferences.

> Did you see that Vikas created bug 553564 to follow up from bug 553472 in
> what I called the mid-term solution?

Ah, thanks for that hint. Maybe I can try to have a look at this. It should probably be a predecessor to further work on 548069.

In any case, please have a look and check if this regression fix can be merged until we have anything better...
Comment 4 Stephan Herrmann CLA 2019-12-09 11:44:10 EST
The code change in gerrit looks good for now (we should revisit it after bug 553564 if all that will still be needed).

Would you be able to also provide a test case?

Jenkins build has been retriggered.
Comment 5 Christoph Langer CLA 2019-12-09 14:41:37 EST
(In reply to Stephan Herrmann from comment #4)
> The code change in gerrit looks good for now (we should revisit it after bug
> 553564 if all that will still be needed).
> 
> Would you be able to also provide a test case?
> 
> Jenkins build has been retriggered.

I've uploaded a new patch set with a test that demonstrates the NPE if run without the fixes.

The second build did not show errors.
Comment 6 Christoph Langer CLA 2019-12-09 14:42:25 EST
> I've uploaded a new patch set with a test that demonstrates the NPE if run
> without the fixes.

-> that is, one of the 3 potential NPEs...
Comment 7 Vikas Chandra CLA 2019-12-09 23:04:17 EST
My view is that we should do a re-spin for this issue. In the current context, it would probably means joining the re-spin.
Comment 8 Vikas Chandra CLA 2019-12-09 23:12:17 EST
Going forward, can we add test cases for all the javadocs in module-info files of openJDK? ( probably warrants another bug)
Comment 9 Eclipse Genie CLA 2019-12-10 00:12:03 EST
New Gerrit change created: https://git.eclipse.org/r/154159
Comment 10 Vikas Chandra CLA 2019-12-10 00:12:56 EST
(In reply to Eclipse Genie from comment #9)
> New Gerrit change created: https://git.eclipse.org/r/154159
 
R4_14_maintenance branch.
Comment 11 Vikas Chandra CLA 2019-12-10 04:40:21 EST
We can put this fix in the next 4.15 I build. 

>>As a workaround you may have to disable javadoc analysis in the preferences.

This can be used for 4.14
Comment 13 Vikas Chandra CLA 2019-12-16 03:27:00 EST
Ideally the change in Javadoc.java should be reverted once Bug 553564 is fixed.

The next I build should be usable fir OpenJDK usecase