Bug 190748 - [1.5][compiler] AbstractMethodError on derived implementation of derived Interface declaration
Summary: [1.5][compiler] AbstractMethodError on derived implementation of derived Inte...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P2 blocker (vote)
Target Milestone: 3.3 RC4   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-04 04:31 EDT by Jan Hoppe CLA
Modified: 2007-06-14 02:13 EDT (History)
2 users (show)

See Also:
philippe_mulet: review+
jerome_lanneluc: review+
maxime_daniel: review+
Olivier_Thomann: review+


Attachments
Error Project with test case (3.41 KB, application/x-java-archive)
2007-06-04 04:36 EDT, Jan Hoppe CLA
no flags Details
Proposed patch (2.83 KB, patch)
2007-06-04 13:01 EDT, Kent Johnson CLA
no flags Details | Diff
Updated patch (2.77 KB, patch)
2007-06-05 10:32 EDT, Kent Johnson CLA
no flags Details | Diff
Correct updated patch (2.87 KB, patch)
2007-06-05 11:12 EDT, Kent Johnson CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Hoppe CLA 2007-06-04 04:31:15 EDT
Since 3.3M6 the following implementation fails with an AbstractMethodError 

interface IBase 
{
	IBaseReturn get();
}

interface IEnhanced extends IBase 
{
	IEnhancedReturn get();
}

abstract class AImpl 
{

	public IEnhancedReturn get() 
	{
		return null;
	}
}

class Impl extends AImpl implements IBase, IEnhanced 
{
}

class ImplTest extends TestCase 
{
	public void testGet() throws Exception 
	{
		IBase base = new Impl();
		IBaseReturn baseReturn = base.get();
		if (baseReturn != null) 
		{
			System.out.println(baseReturn);
		}
	}
}

Test fails:

java.lang.AbstractMethodError: Impl.get()LIBaseReturn;
	at ImplTest.testGet(ImplTest.java:9)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:585)
	at junit.framework.TestCase.runTest(TestCase.java:164)
	at junit.framework.TestCase.runBare(TestCase.java:130)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:120)
	at junit.framework.TestSuite.runTest(TestSuite.java:230)
	at junit.framework.TestSuite.run(TestSuite.java:225)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)


It works on 3.2.x and until 3.3M5 but never again since M6!
Comment 1 Jan Hoppe CLA 2007-06-04 04:36:12 EDT
Created attachment 69926 [details]
Error Project with test case
Comment 2 Philipe Mulet CLA 2007-06-04 07:46:29 EDT
Kent - smells like a late show stopper (now if too risky at this point in the game...). But given it looks like a regression, we probably should address it.
Comment 3 Philipe Mulet CLA 2007-06-04 07:47:10 EDT
Maxime - can you also investigate ? 
Comment 4 Philipe Mulet CLA 2007-06-04 07:48:36 EDT
Tagging tentatively for RC4 (no commitment, just so we do keep track of it).
Comment 5 Kent Johnson CLA 2007-06-04 13:01:24 EDT
Created attachment 69988 [details]
Proposed patch

The optimization to remove inherited methods meant we didn't add a bridge method for IBase.get() to AImpl.

From change for bug 184293
Comment 6 Kent Johnson CLA 2007-06-04 13:33:30 EDT
Risk:
The change is straight forward and has a low risk of causing new problems.

The fix for bug 184293 removed overridden inherited methods for all tests, but should have only done it for checkInheritedReturnTypes() so bridge methods could still be created.

Benefits:
This is a regression from 3.3M6. Its a real case from actual code, so the fix is necessary for some users.
Comment 7 Olivier Thomann CLA 2007-06-04 13:38:59 EDT
+1
Comment 8 Olivier Thomann CLA 2007-06-04 13:39:20 EDT
I didn't mean to close it as FIXED.
Comment 9 Maxime Daniel CLA 2007-06-05 06:45:14 EDT
Fix looks OK. Somehow stresses the fact that the check* series of methods has side effects beyond emitting error messages (which may call for an explicit comment, since the more general accepted meaning of check is: go verify something and return the result - implicitly without changing the observed system).
Hence +1.

Otherwise, we normally no more add tests to the Compliance* classes, but use a test on the compliance instead when results depend on the compliance level. The Compliance* classes should vanish when the migration of existing tests (which may take a while) will be over. I would then suggest that you move the new test to MethodVerifyTest.
Comment 10 Kent Johnson CLA 2007-06-05 10:32:25 EDT
Created attachment 70141 [details]
Updated patch

Moved test to MethodVerifyTest
Comment 11 Kent Johnson CLA 2007-06-05 11:12:06 EDT
Created attachment 70147 [details]
Correct updated patch
Comment 12 Philipe Mulet CLA 2007-06-05 13:26:22 EDT
+1 for 3.3rc4
Comment 13 Jerome Lanneluc CLA 2007-06-05 13:29:43 EDT
+1 for 3.3 RC4
Comment 14 Kent Johnson CLA 2007-06-05 13:31:00 EDT
Released into HEAD for 3.3RC4
Comment 15 Olivier Thomann CLA 2007-06-06 19:24:09 EDT
Verified for 3.3RC4 using I20070606-0010
Comment 16 Jan Hoppe CLA 2007-06-14 02:13:04 EDT
Great work! 
I checked your fix with RC4 in my "real life" application and no more "AbstractMethodErrors" has been throws. 

You made my day!
Jan