Bug 255452 - [1.5][compiler] Eclipse allows forward reference in enum constructor
Summary: [1.5][compiler] Eclipse allows forward reference in enum constructor
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.4.2   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 265307 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-11-16 06:49 EST by Andrey Loskutov CLA
Modified: 2009-05-11 17:44 EDT (History)
4 users (show)

See Also:
jerome_lanneluc: review+


Attachments
Proposed patch (13.76 KB, patch)
2008-11-17 11:23 EST, Philipe Mulet CLA
no flags Details | Diff
Better patch (30.84 KB, patch)
2008-11-18 15:21 EST, Philipe Mulet CLA
no flags Details | Diff
Proposed patch for 3.4 (15.79 KB, patch)
2008-11-18 17:53 EST, Philipe Mulet CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2008-11-16 06:49:42 EST
Build ID: 3.4.1  Build id: M20080911-1700

This code doesn't compile (OK!) in Eclipse (both 1.5/1.6 compilance level), javac 1.5 and javac 1.6:

enum BadEnum {
    CRAZY(CRAZY); // <-- illegal forward reference reported by all compilers
    final BadEnum self;
    private BadEnum(BadEnum self) {
        this.self = self;        
    }
}

This is expected behavior.

However, the code below compiles in Eclipse(both 1.5/1.6 compilance level) and javac 1.5, but NOT in javac 1.6:

enum Impossible {
    CRAZY(Impossible.CRAZY); // <-- illegal forward reference (javac 1.6 only)
    final Impossible self;
    private Impossible(Impossible self) {
        this.self = self;        
    }
}

I guess this is a bug in both Eclipse and javac 1.5 compilers, which is fixed only in javac 1.6.
Comment 1 Philipe Mulet CLA 2008-11-17 05:25:38 EST
I kind of remember that qualified access was ok, since it would pick a default value if unitialized. Need to check.

>I guess this is a bug in both Eclipse and javac 1.5 compilers, which is fixed
only in javac 1.6.
Did you find a bug reference, this might help in decision.
Comment 2 Philipe Mulet CLA 2008-11-17 05:41:03 EST
Indeed enums are a bit special. Similar offending code is tolerated for classes:

enum BadEnum {
    CRAZY(CRAZY), // <-- illegal forward reference reported by all compilers
    IMPOSSIBLE(BadEnum.IMPOSSIBLE); // <-- illegal forward reference (javac 1.6 only)
    private BadEnum(BadEnum self) {
    }
}

public class X {
	X x1 = new X(x1);//1 - WRONG
	static X X2 = new X(X.X2);//2 - OK
	X x3 = new X(this.x3);//3 - OK
	X(X x) {
	}
}
class Y extends X {
	X x1 = new X(x1);//4 - WRONG
	static X X2 = new X(Y.X2);//5 - OK
	X x3 = new X(this.x3);//6 - OK
	Y(Y y) {
		super(y);
	}
}
Comment 3 Andrey Loskutov CLA 2008-11-17 07:10:28 EST
I guess it can be
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6424491
but I don't know sure. 

I've just found a bug in my code, wondered why it is not rejected by compiler (final enum value will be null with the "bad" code, which is clearly not intendend to be), then I've compared different compilers and saw that sun's 1.6 javac does it "the right way" I would expect as a user, but Eclipse doesn't. 

Because both 1.5 javac and Eclipse does the "wrong thing", I've marked this as a [1.6] level issue (to keep ECJ compatibility with 1.5 javac).
Comment 4 Philipe Mulet CLA 2008-11-17 11:23:09 EST
Created attachment 118059 [details]
Proposed patch
Comment 5 Philipe Mulet CLA 2008-11-17 11:24:41 EST
The fix is also enabled in 1.5 compliant mode, since this is just a plain bug, which they only addressed from 1.6 on.
Comment 6 Philipe Mulet CLA 2008-11-18 15:21:54 EST
Created attachment 118188 [details]
Better patch
Comment 7 Philipe Mulet CLA 2008-11-18 15:49:03 EST
Released for 3.5M4.
Fixed

Comment 8 Philipe Mulet CLA 2008-11-18 17:53:50 EST
Created attachment 118206 [details]
Proposed patch for 3.4
Comment 9 Philipe Mulet CLA 2008-11-18 17:55:56 EST
Candidating for 3.4.2.

Symptoms are bad. We incorrectly accept invalid code, and the fix is quite simple (need one more check for subsequent bindings in qualified name reference).

The patch also improves slightly the error message error position (on the offending portion of the qualified name).
Comment 10 Jerome Lanneluc CLA 2008-11-19 05:47:47 EST
Naive question: is there a risk that fieldBinding.original().declaringClass can be null?
Comment 11 Philipe Mulet CLA 2008-11-19 16:31:24 EST
If we hadn't resolved the fieldBinding we would have bailed out before that point.
Comment 12 Jerome Lanneluc CLA 2008-11-27 05:00:43 EST
+1 for 3.4.2
Comment 13 Jerome Lanneluc CLA 2008-12-09 05:43:51 EST
Verified for 3.5M4 using I20081209-0100
Comment 14 Kent Johnson CLA 2009-02-03 12:22:27 EST
Verified for 3.4.2 using M20090127-1710
Comment 15 Kent Johnson CLA 2009-02-18 16:58:45 EST
*** Bug 265307 has been marked as a duplicate of this bug. ***
Comment 16 Olivier Thomann CLA 2009-05-11 17:44:23 EDT
*** Bug 275736 has been marked as a duplicate of this bug. ***