Bugzilla – Full Text Bug Listing
| Summary: | [compiler] unnecessary null-checks in ExplicitConstructorCall.resolve | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stephan Herrmann <stephan.herrmann> |
| Component: | Core | Assignee: | Stephan Herrmann <stephan.herrmann> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | minor | ||
| Priority: | P3 | CC: | philippe_mulet, stephan.herrmann |
| Version: | 3.4 | ||
| Target Milestone: | --- | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | stalebug | ||
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. If you have further information on the current state of the bug, please add it. 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 ID: I20080617-2000 I noticed some inconsistency wrt null-checking in method ExplicitConstructorCall.resolve: The local variable receiverType is checked for null on several paths except for line 383: } else if (receiverType.erasure().id == TypeIds.T_JavaLangEnum) { although there are null-checks both above and below. However, I did not succeed in constructing a test that would actually break in this line (except that a bug in our branch of the jdt.core had actually produced an NPE in this line :-/ ). So I inserted a check immediately after the two assignments to receiverType such as to throw an exception if null is actually encountered. I ran several test-suites, specifically org.eclipse.jdt.core.tests.compiler.regression (package) org.eclipse.jdt.apt.pluggable.tests org.eclipse.jdt.compiler.apt.tests None of these were affected by the forced exception. (these tests used our branch, so a *minimal* chance exists, that we're actually masking a problem here - sorry, I don't have a test setup ready to run all the tests against a pristine jdt). Assuming that clarity in this question might help exposing existing design choices I browsed the sources including old versions. This is my conclusion: 1. One initial null-check was present right from cvs version 1.1 of ExplicitConstructorCall, so we can't reproduce whether a null ever occurred: at any time in history it would have been handled, rather than throwing an exception. 2. scope.enclosingReceiverType() should be assumed to never return null when resolve has already started. 3. receiverType.superclass() can actually return null, but *only* for class java.lang.Object, which is already handled in ConstructorDeclaration.resolveStatements, or for interfaces which don't have constructors. Another way to trick the compiler would have been by a broken classpath such that types with implicit superclass java.lang.Object would indeed have a null-superclass. Before the patch for bug 114349 (v_730) this situation would have caused the compilation to abort, after that patch a MissingBinaryTypeBinding would have been inserted (since bug 196200: a MissingTypeBinding). 4. The patch for bug 196200 carefully moved the null-check down into several branches (except for the one mentioned) (no, the patch didn't do anything, it was Phillipe doing this ;-) In order to manifest these findings in the code I would actually suggest to remove the existing null-checks (lines 291, 356-358, 387-389). If removing null-checks is not against any general policy, I'd assume now (= shortly after a release) would be a good point in time for doing so. Also ReferenceBinding.superclass() might be documented saying that after completeTypeBindings() null is never returned. Then, if we'd actually see an NPE, we knew it's unintended behavior that should be fixed upstream. (That's actually, why I had to dig into this: I saw a null and only after this investigation I was sure that the null was wrong in the first place).