| Summary: | NPE in ElementImpl.hashCode (again) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stephen Haberman <stephen> | ||||
| Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | amj87.iitr, eclipse, Olivier_Thomann, srikanth_sankaran | ||||
| Version: | 3.6 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | stalebug | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 320832 | ||||||
| Attachments: |
|
||||||
Reassigning to JDT core: problem is happening because the Binding passed into the ElementImpl constructor is null, instead of being an unresolved reference binding. JDT core guys, would you like me to put together a test case for this, or do you have enough information in the bug report? Assign to me if you'd like a test case. Satyam, please follow up. Thanks! SourceTypeBinding#resolveTypesFor(MethodBinding) is intended to set the methodbinding to null if it is not resolved. AnnotationDiscoveryVisitor#visit(MethodDeclaration, ClassScope) is doing a null check for the binding and then calling the resolveTypeFor(). Just doing a null check even after resolveTypesFor() should probably be enough. Do you see any problems with it? Yes, if the compiler is intending to give us null bindings there then we'll need to be more defensive in the AnnotationDiscoveryVisitor. What is the rationale for when a null binding is returned versus when an UnresolvedReferenceBinding is returned? As I understand, asking for binding using functions like findMethod() returns ProblemMethodBinding. resolvedTypesFor() updates the internal data structures to null. To check the validity of a binding, both the null check and validity check for the binding needs to be done like (methodBinding != null && methodBinding.isValidBinding()) Moving to JDT/APT, because the fix should be in the APT code. Created attachment 173672 [details]
Proposed patch
Attached proposed patch, for discussion. Not yet tested.
I'll investigate why we don't get a missing type binding in this case. Srikanth, I think the difference is that for ParameterizedQualifiedTypeReference we first such for a package binding and we cannot resolve it. So we get a ProblemTypeBinding back that contains a missing type binding as its closest match. Now the question is what the "best" answer for this binding can be. If we decide that null is the best answer, we will need to make more checks inside the visitor for potential null bindings. Surprisingly enough, I can get a pretty decent binding in DOM/AST with recovered bindings. So to be clear, it is not sure we can improve that case, but we have several cases where missing types inside parameterized type references are an issue. See also bug 320633. For 320633, I need to investigate what is done with binding recovery. This requires a deep analysis and therefore I remove the target to 3.6.1. Maybe we should put the null check in AST visitor, for this particular case, for 3.6.1? (In reply to comment #9) > Maybe we should put the null check in AST visitor, for this particular case, > for 3.6.1? Then we should clone the bug. It is unlikely we will do a risky change for 3.6.1, so it doesn't make sense to me to keep the same bug report for both streams as we won't end up with the same fix on both streams if we find a way to improve the missing type binding support. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |
Build Identifier: I20100608-0911 With APT enabled, methods annotated with SuppressWarnings, a generic parameter, and an unresolved inner type will cause an NPE compiler crash. Reproducible: Always Steps to Reproduce: Put the following code in an APT-enabled project: public class Foo { @SuppressWarnings("unchecked") private <B> void bar(final Zaz.Inner<B> zaz) { } } Where Zaz is an unresolved type and you should get the following stack trace [1]. Note that changing Zaz.Inner<B> to "Zaz<B>" or "Zaz.Inner" both make the NPE go away. As does removing the SuppressWarnings, or disabling the APT processor. Or changing "Zaz" to be a resolved type (say java.util.Map or fixing the classpath error that led to Zaz being unavailable). [1]: java.lang.NullPointerException at org.eclipse.jdt.internal.compiler.apt.model.ElementImpl.hashCode(ElementImpl.java:110) at java.util.HashMap.put(HashMap.java:372) at java.util.HashSet.add(HashSet.java:200) at org.eclipse.jdt.internal.compiler.apt.util.ManyToMany.put(ManyToMany.java:198) at org.eclipse.jdt.internal.compiler.apt.dispatch.AnnotationDiscoveryVisitor.resolveAnnotations(AnnotationDiscoveryVisitor.java:187) at org.eclipse.jdt.internal.compiler.apt.dispatch.AnnotationDiscoveryVisitor.visit(AnnotationDiscoveryVisitor.java:128) at org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.traverse(MethodDeclaration.java:230) at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1290) at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:691) at org.eclipse.jdt.internal.compiler.apt.dispatch.RoundEnvImpl.<init>(RoundEnvImpl.java:56) at org.eclipse.jdt.internal.compiler.apt.dispatch.BaseAnnotationProcessorManager.processAnnotations(BaseAnnotationProcessorManager.java:148) at org.eclipse.jdt.internal.apt.pluggable.core.dispatch.IdeAnnotationProcessorManager.processAnnotations(IdeAnnotationProcessorManager.java:134) at org.eclipse.jdt.internal.compiler.Compiler.processAnnotations(Compiler.java:809) at org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:428) at org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.compile(AbstractImageBuilder.java:364) at org.eclipse.jdt.internal.core.builder.BatchImageBuilder.compile(BatchImageBuilder.java:178) at org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.compile(AbstractImageBuilder.java:301) at org.eclipse.jdt.internal.core.builder.BatchImageBuilder.build(BatchImageBuilder.java:60) at org.eclipse.jdt.internal.core.builder.JavaBuilder.buildAll(JavaBuilder.java:254) at org.eclipse.jdt.internal.core.builder.JavaBuilder.build(JavaBuilder.java:173) at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:629) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:172) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:203) at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:255) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:258) at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:311) at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:343) at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:144) at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:242) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)