Community
Participate
Working Groups
Created attachment 232173 [details] Class that compiles to different override behaviors between javac and eclipse (4.3RC3) When compiling the attached java file using javac and eclipse 4.2, no compilation errors occur. Using Eclipse 4.3RC3 (4.3M6 also has this issue) gives a compilation error: The type Main.SubSub must implement the inherited abstract method Main.Base<Main.F3>.foo(Main.F3) Main.java(42) Removing the 'abstract' keyword from the Main#Sub class also removes the compilation error, as does implementing a `foo(F3 f)` method. The Eclipse generated byte code results in different runtime behavior when compared to the output generated by the javac compiler. An implementation of the foo(F3) method in the SubSub type (with the Sub type remaining abstract): @Override public void foo(F3 bar) { System.out.println(getClass().getSimpleName() + ": F3 + " + bar.getClass().getSimpleName()); bar.bar(); } The expected result as given by javac/java SubSub: F3 + F3 bar in F3 SubSub: F3 + F3 bar in F3 SubSub: F3 + F3 bar in F3 SubSub: F2 + F2 The result as given by the eclipse compiler: SubSub: F3 + F3 bar in F3 SubSub: F2 + F3 SubSub: F3 + F3 bar in F3 SubSub: F2 + F2 As you can see the call to 'sub2.foo(f3);' performed in Main#main(), line 11 gives a strange result, whereas the call in line 12 does call the correct function (giving the expected result when compared to javac) Commandline java version: java version "1.7.0_21" Java(TM) SE Runtime Environment (build 1.7.0_21-b12) Java HotSpot(TM) 64-Bit Server VM (build 23.21-b01, mixed mode)
Stephan, looks like a candidate for 4.3.1. Can you please take a look?
ack
I can reproduce the different program output when compiled with ecj vs. javac. (In reply to comment #0) > Using Eclipse 4.3RC3 (4.3M6 also has this issue) > gives a compilation error: > > The type Main.SubSub must implement the inherited abstract method > Main.Base<Main.F3>.foo(Main.F3) Main.java(42) This I cannot reproduce. All versions >= 3.6 (!) compile the program without any error. Earlier versions report: The method foo(Main.F3) of type Main.SubSub must override or implement a supertype method Note, that I could also not reproduce any difference between ecj versions within the range 3.6 - 4.3RC4. Without evidence that ecj behavior indeed changed this doesn't classify as a regression and fixing for Luna might be sufficient. Am I missing anything?
This is the distribution I downloaded that exhibits the compiler error: > Eclipse Java EE IDE for Web Developers. > > Version: Kepler Release > Build id: 20130606-0932 > > (c) Copyright Eclipse contributors and others 2005, 2013. All rights reserved. > Visit http://www.eclipse.org/webtools I'll upload the full project as an attachment (perhaps the project settings are different).
Created attachment 232227 [details] Complete project that shows the behavior from comment #0
Created attachment 232228 [details] Eclipse configuration Added my eclipse configuration for 4.3RC3 (JEE download) to give insight into which plugins are installed.
Thanks, the attachment from comment 5 helps to see: not the project settings make the difference, but whether or not SubSub implements foo :)
With the corrected example I could narrow it down to being a regression caused by commit 4dd974a226271180ff02d909a12722017f80ff3a on behalf of bug 395681. Sigh! Note, however, that the regression only concerns the compile error, not the behavioral difference to javac. In the end we may have to split the fix into fixing the regression (for 4.3.1) and addressing the behavioral difference (for 4.4 - assuming that indeed ecj is wrong, could also be javac).
(In reply to comment #8) > Note, however, that the regression only concerns the compile error, not the > behavioral difference to javac. In the end we may have to split the fix into > fixing the regression (for 4.3.1) and addressing the behavioral difference > (for 4.4 - assuming that indeed ecj is wrong, could also be javac). Stephan, now that we are very close to 4.3.1, as you said, we should perhaps fix only the compiler regression part for 4.3.1 and take a decision on the rest later?
Let me gather some relevant information: The situation triggering the compiler error looks identical to bug 411811. The relevant code portion here is: public static class F1 { } public static class F2 extends F1 { } public static class F3 extends F2 { } public static abstract class Base<T extends F1> { public abstract void foo(T bar); } public static abstract class Sub<T extends F2> extends Base<T> { @Override public void foo(F2 bar) { } } public static class SubSub extends Sub<F3> { } Here the question is, whether SubSub needs its own method to implement Base.foo(T). Looking from SubSub that method spells out as "foo(F3)" and SubSub does not declare nor inherit any concrete method with a compatible signature. That's the error we started reporting with Kepler. In bug 411811 comment 4 I argued at some more detail, why I believe this new behavior to be correct wrt JLS7. Next I asked about this in the JSR 335 EG, because the new draft spec differs in these points, and I wanted to check if the change was intended, accidental or what: http://mail.openjdk.java.net/pipermail/lambda-spec-experts/2013-July/000308.html The last answer in that thread states: "the overriding relation is monotonic with respect to inheritance: you can add new overrides via inheritance, but can't "undo" existing ones." While this is not part of the spec nor explicitly backed by the spec, we should probably take that as the *intended spec*. Unfortunately, the notion of override monotonicity doesn't easily translate into an implementation strategy, because method verification always happens for each type in isolation. We start with a flat set of inherited methods, rather than with the result of method verification for the super class. I'll see if I find an easy solution.
*** Bug 415600 has been marked as a duplicate of this bug. ***
Created attachment 234677 [details] patch under test
Implementation Notes: --------------------- Before the patch, the check isSkippableOrOverridden() directly compared signatures using isParameterSubsignature(specific, general). Since both methods have been instantiated from the perspective of the focus type (SubSub in the problem case), we didn't find a match. Using isMethodSubsignature(specific, general) instead, we now use 'specific.original()' and adjust 'general' accordingly. This reflects the perspective from the declaration of 'specific'. Doing so assures that the original overriding relationship (as seen in Sub) is still recognized. This change caused regressions first against bug 346029 later bug 195802. (Cited tests are in MethodVerifyTests). 1) In test346029() we detected too many matches, pulling up the check 'specific.declaringClass.isCompatibleWith(general.declaringClass)' one level solved the regression. Now it looks a bit funny that skip[idx] and isOverridden[idx] are assigned in lock step. Once we are sure this is the right fix, we should check if this can be simplified (outside isSkippableOrOverridden() these arrays are still used independently). 2) Finally test146 (on behalf of bug 195802) reported its original bogus "duplicate" error. Since the original fix can no longer be recognized in the code (due to subsequent restructuring), I freshly special-cased this situation. The situation can be recognized by a pair of methods, where one is the rawified PGMB of the other. I believe this to be a precise description of the case in bug 195802, so directly avoiding the duplication error should be OK.
I have added more tests from duplicates (bug 411811 and bug 415600) - both are green with the patch. I have released the patch for *master* (commit ce089180323dc4b5ef74d43ea0b4475c385fcd97) and would like to ask everybody to test this patch since it touches a brittle area of the compiler. If anybody has the time for some extra testing I suggest to use other bugs for inspiration: the duplicates mentioned above and also bug 195802 and bug 346029, which were broken by intermediate versions of the patch. One idea would be to try to construct some nasty tests by using interfaces where existing tests only use classes. I hope we can get sufficient confidence for back porting in time for SR1 RC2.
+1 for 4.3.1.
(In reply to comment #14) > I hope we can get sufficient confidence for back porting in time for SR1 RC2. I did some testing and no issues so far.
Released for 4.3.1 via commit 288e8d99c513639a6f3fdf977634474ff458e3aa
*** Bug 411811 has been marked as a duplicate of this bug. ***
Verified for 4.3.1 with build M20130828-0800
Founds another case for this still not working. Using Version: 4.3.1, Build id: M20130905-0705. See the attachment: the class Worker3 causes the same error. In v3.7.1 it compiles ok.
Created attachment 235343 [details] Another case, where the solution doesn't work yet.
(In reply to Tomasz Stanczak from comment #21) > Created attachment 235343 [details] > Another case, where the solution doesn't work yet. Can you please open a new bug for this. Thanks.
*** Bug 416917 has been marked as a duplicate of this bug. ***
