Comment 1 Olivier Thomann 2007-05-10 13:44:27 EDT

The problem seems to occur before the RecoveredTypeBinding is reached. For this binding, we set the superclass to java.lang.Object so this should be fine.
The only problem I can see is if we set Object to be the superclass of Object as a  recovered binding. But this would not produce the stack trace we are seeing here.
Martin, any clue?

Comment 2 Olivier Thomann 2007-05-10 18:54:33 EDT

If you get it again, please try to remember what you did.

Comment 3 Martin Aeschlimann 2007-05-11 03:32:53 EDT

I don't know what it could be... 

Comment 4 Olivier Thomann 2007-05-11 10:54:43 EDT

We definitely need a reproducable test case for this one.

Comment 5 Andy Carroll 2007-05-11 15:16:30 EDT

I just realized the analysis I made in the original bug report I filed was flawed.

Stack traces of course, are logged from the from the top of the stack down.... In  otherwords the top line as it appears in log of a stack trace is the deepest method called, while the next line is the caller of that function etc.

Since the first line reported in the log for the stack trace is HashtableOfPackage.java:51 - presumably that is the line of code being executed when the stack overflow occured.

The problem with this revised reasoning, is that it implies that somehow the JVM spun in an recursive loop for around 1000 iterations at Bindings.java:446, until it miraculously broke free of this loop, only to then run out of stack space a mere 8 stack frames deeper.... 

NOTE: While for the sake of brievity I didn't attach the full log - there is absolutely nothing else logged other than approx 1000 lines to calls to Bindings.java:446 - there's no hint as to the root caller of Bindings.findOverriddenMethodInHierarchy

Since none of this makes sense, I assume that a side effect is of a StackOverflowError is that the stack trace that is logged is misleading in some respect.... 

Anyway, while I doubt this revised reasoning is helpful, I thought I should mention it, incase my initial flawed report was steering analysis of the problem down a wrong path.

If it happens to me again, I'll update this issue with more information.

Comment 6 Olivier Thomann 2007-05-11 17:15:14 EDT

Thanks for the additional info. But it doesn't change much since we need to understand the 1000's call to getSuperclass().
For now there is no much I can do for RC1. But if we get a reproducable test case, we might be able to fix it post RC1.

Comment 7 Olivier Thomann 2007-08-01 14:33:27 EDT

Closing as WORKSFORME.
I could never get a reproducable test case.
Please reopen if you get it again.

Comment 8 Frederic Fusier 2007-08-07 05:58:49 EDT

Verified for 3.4M1 using build I20070806-1800.

Comment 9 Markus Keller 2007-11-07 06:49:32 EST

Steps to reproduce in I20071106-0816:

- paste to Package Explorer:

package p;
public class A {
	void m() { }
}
//--
package p;
public class C extends A {
	void m() {}
}

- remove the 'JRE System Library' from the build path
- go to Java Browsing perspective
- open p.A (by using the browsing views)
- open p.C (by using the browsing views)
- open p.A (by using the browsing views)
=> boom

The type binding for p.A from the ASTProvider has an invalid superclass:
SUPERCLASS: Object
	NAME: 'Object'
	KEY: 'Recovered#referenceBindingLjava/lang/Object;0<>'
	IS RECOVERED: true
	QUALIFIED NAME: 'Object'
	KIND: isClass
	GENERICS: (non-generic, non-parameterized)
	CREATE ARRAY TYPE (+1): Object[]
	ORIGIN: isTopLevel
	IS FROM SOURCE: false
	PACKAGE: p
	DECLARING CLASS: null
	DECLARING METHOD: null
	MODIFIERS: (empty string)
	BINARY NAME: null
	TYPE DECLARATION: Object
	ERASURE: Object
	SUPERCLASS: Object // this is the same invalid 'Object' binding again
	INTERFACES (0)
	DECLARED TYPES (0)
	DECLARED FIELDS (0)
	DECLARED METHODS (0)
	IS SYNTHETIC: false
	IS DEPRECATED: false
	ANNOTATIONS (0)
	> CompilationUnit: p.Object (does not exist)

Comment 10 Markus Keller 2007-11-07 06:49:53 EST

*** Bug 208997 has been marked as a duplicate of this bug. ***

Comment 11 Frederic Fusier 2007-11-07 07:22:00 EST

I can now reproduce the problem with 3.4M3 and scenario described at comment 9.

Comment 12 Frederic Fusier 2007-11-07 09:53:46 EST

*** Bug 209031 has been marked as a duplicate of this bug. ***

Comment 13 Frederic Fusier 2007-11-07 10:11:02 EST

Created attachment 82336 [details]
Proposed patch

Olivier, can you review this patch?
Thanks

Comment 14 Olivier Thomann 2007-11-07 11:57:09 EST

Jérôme, this should be backported for 3.3.2.

Comment 15 Markus Keller 2007-11-07 12:35:54 EST

The recovered 'Object' type binding is still a bit strange, since it has name "Object" and a package binding with name "p" but a qualifiedName "Object".

Expected: either qualifiedName "p.Object", or better yet "java.lang.Object" and a package binding for "java.lang" (since ITypeBinding.getSuperclass() suggests to test for "java.lang.Object").

Comment 16 Olivier Thomann 2007-11-07 12:58:41 EST

In this specific case, we could indeed do better.
We retrieve a missing binary type binding that contains the right package binding and the right fully qualified name.
So we should improve:
-getPackage() to return the package binding corresponding to the package binding inside the missing binary type binding. This will involve updating the doc for ITypeBinding#getPackage().
- getQualifiedName() to return a consistent name wrt the package name
- getName() same for the single name

Of course getSuperclass() should also be fixed, but for this, we have a fix already.

Comment 17 Frederic Fusier 2007-11-08 05:05:12 EST

I opened bug 209510 for problem noticed by Markus at comment 15...

Comment 18 Frederic Fusier 2007-11-08 07:06:31 EST

Created attachment 82427 [details]
Better proposed patch

This patch uses new implemented RecoveredTypeBinding.equals(Object) method instead of identity test. Added new test using code illustrating this peculiar need.
Also move the test cases for this bug in a new test suite as this kind of tests do not require the Converter* projects to be copied...

Olivier, please review it again, thanks

Comment 19 Frederic Fusier 2007-11-08 10:26:36 EST

equals(Object) is too complicated to fix this issue. Combined with bug 209150, we can have a simpler solution to test equality: getQualifiedName().equals("java.lang.Object")

I'll be back soon with a new patch...

Comment 20 Frederic Fusier 2007-11-08 11:02:37 EST

Created attachment 82455 [details]
New proposed patch

Comment 21 Olivier Thomann 2007-11-08 11:19:01 EST

+1.
Should be backported to 3.3.2.

Comment 22 Frederic Fusier 2007-11-08 12:51:44 EST

The patch has issues with array types. I need to rewrite it a little bit...

Comment 23 Frederic Fusier 2007-11-08 12:54:14 EST

Created attachment 82466 [details]
Last (?) proposed patch

ITypeBinding.getPackage() API specifies that it returns null for array type. This patch do the right thing now. It also fixes problem with missing [] in array types qualified name (I've added a new test for this).

Also (Markus) updated the javadoc comment for getPackage() method...

Comment 24 Frederic Fusier 2007-11-08 13:46:10 EST

Created attachment 82472 [details]
Last proposed patch fixed by Olivier

Olivier modified my previous patch after his review. getReferenceBinding() method of RecoveredTypeBinding now returns the reference of the most inner type if any.
This allow to simplify the getQualifiedName() method and make it safer.
Thanks Olivier

Comment 25 Frederic Fusier 2007-11-08 13:46:39 EST

Released for 3.4M4 in HEAD stream.

Comment 26 Jerome Lanneluc 2007-11-13 05:35:03 EST

+1 for backporting to 3.3.2

Comment 27 Frederic Fusier 2007-11-13 07:28:40 EST

Created attachment 82754 [details]
Proposed patch for 3.3.2

Comment 28 Frederic Fusier 2007-11-13 07:33:27 EST

*** Bug 208997 has been marked as a duplicate of this bug. ***

Comment 29 Frederic Fusier 2007-11-13 10:08:39 EST

Released for 3.3.2 in R3_3_maintenance stream.

Comment 30 Jerome Lanneluc 2007-11-21 05:24:43 EST

Since there is no workaround and a StackOverFlowError is always bad, this fix needs to be included in 3.3.2. Philippe please approve.

Comment 31 Philipe Mulet 2007-11-21 12:31:26 EST

+1 for 3.3.2

Comment 32 Frederic Fusier 2007-11-21 13:55:25 EST

*** Bug 210052 has been marked as a duplicate of this bug. ***

Comment 33 Eric Jodet 2007-12-12 03:32:51 EST

Verified for 3.4 M4 using build I20071211-0010

Comment 34 Olivier Thomann 2008-06-13 09:03:40 EDT

*** Bug 237022 has been marked as a duplicate of this bug. ***