Community
Participate
Working Groups
REPRODUCING THE PROBLEM: 1) Create a workspace with one java project. In the Java-Compiler "Errors/Warning" -settings turn on "enabled annotation-based null-analysis". 2) Add the following two classes: --- ClassA.java: import org.eclipse.jdt.annotation.NonNullByDefault; @NonNullByDefault public class ClassA { public interface InterfaceA { public void method(Object arg); } } --- ClassB.java: import org.eclipse.jdt.annotation.NonNull; public class ClassB { public static class ImplA implements ClassA.InterfaceA { @Override public void method(@NonNull Object arg) { } } } 3) Now close all Editor windows. 4) Chose Project > Clean ... > Clean all projects: THE PROBLEM: The following error appears in the problems view: "Illegal redefinition of parameter arg, inherited method from ClassA.InterfaceA does not constrain this parameter", ClassB.java, line 6 ANALYSIS: In SourceTypeBinding.java, the field defaultNullness is accessed in the method hasNonNullDefault(), but was not initialized for some of the involved classed. (There is another reference to it in checkRedundantNullnessDefaultOne, which is not resposible for the bug in the example, but looks suspicous too) Invoking initializeNullDefault() before accessing defaultNullness fixes the problem. I will attach a patch which does that in both locations.
Created attachment 234505 [details] proposed patch
This contribution complies with http://www.eclipse.org/legal/CoO.php
I forgot: obviously you have to configure the build path so it contains the null-annotations before step 4)
this bug should probably be assigned to stephan herrmann
How is Juno coping with this scenario? Is this a regression in Kepler? I see that the initialization can be incomplete in specific situations. However, changing order of processing steps always bears the risk of side-effects in unexpected locations, so I'm targeting this for Luna, not Kepler SR1. OTOH, initializeNullDefault() is already designed to support lazy initialization, so inserting a few more calls *may* be the natural, good thing to do. Maybe the real solution will be to make the dependency outer-inner explicit also via the stages of nullnessDefaultInitialized.
I just tried it, it is already broken in Juno. I tried to make a minimal patch that fixes the bug. For a cleaner solution, the field defaultNullness should either be final and set in the constructor, or it should only be accessed via a getter that handles the lazy initialization or at least has an assert-check that checks that nullnessDefaultInitialized has the right state. Also, there is workaround: I add "@SuppressWarnings("null") @NonNull" to all params of interfaces declared as part of classes, and the compiler is set to ignore "Unused '@SuppressWarnings' token": @NonNullByDefault public class ClassA { public interface InterfaceA { public void method(@SuppressWarnings("null") @NonNull Object arg); } } So once a eclipse which fixes this bug is released, i can search for "@SuppressWarnings("null") @NonNull" and remove it.
Created attachment 234679 [details] test case fails without patch, works with proposed patch.
Test and fix are good. The fix renders a few existing calls to initializeNullDefault() redundant, but I decided to keep redundant calls for the sake of comprehensibility. If a call is indeed redundant it only consists of an empty switch, so it shouldn't cause any harm. Released for 4.4 M2 via commit 87621a357241c8de8ae7bf1f07316036b31e3bf8 Thanks, Till!
(In reply to Till Brychcy from comment #6) > For a cleaner solution, the field defaultNullness should either be final and > set in the constructor, We are not ready for computing the defaultNullness when the constructor is executed. > ... or it should only be accessed via a getter that > handles the lazy initialization or at least has an assert-check that checks > that nullnessDefaultInitialized has the right state. Rereading this and thinking about redundant calls I came up with a combined improvement: - removed one call to initializeNullDefault() before hasNonNullDefault(), because the latter now already ensures initialization - changed initializeNullDefault to getNonNullDefault and adjusted clients accordingly This now looks a clean solution to me. Released via commit 1538facbe020906d8e9620bcca922106c178a710
Verified for SDK-I20130916-2330
Verified for 4.4. M2 with build I20130916-2330.
*** Bug 418233 has been marked as a duplicate of this bug. ***
Could the fix be applied to Kepler SR2 or is it still considered too risky?
(In reply to Marc-Andre Laperle from comment #13) > Could the fix be applied to Kepler SR2 or is it still considered too risky? Ignore my comment, it is much too late for SR2.
